| Message ID | 20191013071824.222946-1-morbo@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests,1/1] x86: use pointer for end of exception table | expand |
On 13/10/2019 09.18, Bill Wendling wrote: > Two global objects can't have the same address in C. Clang uses this > fact to omit the check on the first iteration of the loop in > check_exception_table. > > Signed-off-by: Bill Wendling <morbo@google.com> > --- > lib/x86/desc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/x86/desc.c b/lib/x86/desc.c > index 451f504..cfc449f 100644 > --- a/lib/x86/desc.c > +++ b/lib/x86/desc.c > @@ -41,7 +41,7 @@ struct ex_record { > unsigned long handler; > }; > > -extern struct ex_record exception_table_start, exception_table_end; > +extern struct ex_record exception_table_start, *exception_table_end; > > static const char* exception_mnemonic(int vector) > { > @@ -113,7 +113,7 @@ static void check_exception_table(struct ex_regs *regs) > (((regs->rflags >> 16) & 1) << 8); > asm("mov %0, %%gs:4" : : "r"(ex_val)); > > - for (ex = &exception_table_start; ex != &exception_table_end; ++ex) { > + for (ex = &exception_table_start; ex != (void*)&exception_table_end; ++ex) { > if (ex->rip == regs->rip) { > regs->rip = ex->handler; > return; > That looks like quite an ugly hack to me - and if clang gets a little bit smarter, I'm pretty sure it will optimize this away, too. Could you please check if something like this works, too: diff --git a/lib/x86/desc.c b/lib/x86/desc.c index 451f504..0e9779b 100644 --- a/lib/x86/desc.c +++ b/lib/x86/desc.c @@ -41,7 +41,8 @@ struct ex_record { unsigned long handler; }; -extern struct ex_record exception_table_start, exception_table_end; +extern struct ex_record exception_table_start; +extern long exception_table_size; static const char* exception_mnemonic(int vector) { @@ -108,12 +109,13 @@ static void check_exception_table(struct ex_regs *regs) { struct ex_record *ex; unsigned ex_val; + int cnt = exception_table_size / sizeof(struct ex_record); ex_val = regs->vector | (regs->error_code << 16) | (((regs->rflags >> 16) & 1) << 8); asm("mov %0, %%gs:4" : : "r"(ex_val)); - for (ex = &exception_table_start; ex != &exception_table_end; ++ex) { + for (ex = &exception_table_start; cnt-- > 0; ++ex) { if (ex->rip == regs->rip) { regs->rip = ex->handler; return; diff --git a/x86/flat.lds b/x86/flat.lds index a278b56..108fad5 100644 --- a/x86/flat.lds +++ b/x86/flat.lds @@ -9,6 +9,7 @@ SECTIONS exception_table_start = .; *(.data.ex) exception_table_end = .; + exception_table_size = exception_table_end - exception_table_start; } . = ALIGN(16); .rodata : { *(.rodata) } Thomas
On Tue, Oct 15, 2019 at 12:41 AM Thomas Huth <thuth@redhat.com> wrote: > > On 13/10/2019 09.18, Bill Wendling wrote: > > Two global objects can't have the same address in C. Clang uses this > > fact to omit the check on the first iteration of the loop in > > check_exception_table. > > > > Signed-off-by: Bill Wendling <morbo@google.com> > > --- > > lib/x86/desc.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/lib/x86/desc.c b/lib/x86/desc.c > > index 451f504..cfc449f 100644 > > --- a/lib/x86/desc.c > > +++ b/lib/x86/desc.c > > @@ -41,7 +41,7 @@ struct ex_record { > > unsigned long handler; > > }; > > > > -extern struct ex_record exception_table_start, exception_table_end; > > +extern struct ex_record exception_table_start, *exception_table_end; > > > > static const char* exception_mnemonic(int vector) > > { > > @@ -113,7 +113,7 @@ static void check_exception_table(struct ex_regs *regs) > > (((regs->rflags >> 16) & 1) << 8); > > asm("mov %0, %%gs:4" : : "r"(ex_val)); > > > > - for (ex = &exception_table_start; ex != &exception_table_end; ++ex) { > > + for (ex = &exception_table_start; ex != (void*)&exception_table_end; ++ex) { > > if (ex->rip == regs->rip) { > > regs->rip = ex->handler; > > return; > > > > That looks like quite an ugly hack to me - and if clang gets a little > bit smarter, I'm pretty sure it will optimize this away, too. > I agree. I was thinking about this actually and this patch looks like it'll work. I'll generate a new version of the patches. diff --git a/lib/x86/desc.c b/lib/x86/desc.c index 451f504..4002203 100644 --- a/lib/x86/desc.c +++ b/lib/x86/desc.c @@ -113,7 +113,7 @@ static void check_exception_table(struct ex_regs *regs) (((regs->rflags >> 16) & 1) << 8); asm("mov %0, %%gs:4" : : "r"(ex_val)); - for (ex = &exception_table_start; ex != &exception_table_end; ++ex) { + for (ex = &exception_table_start; ex < &exception_table_end; ++ex) { if (ex->rip == regs->rip) { regs->rip = ex->handler; return; > Could you please check if something like this works, too: > > diff --git a/lib/x86/desc.c b/lib/x86/desc.c > index 451f504..0e9779b 100644 > --- a/lib/x86/desc.c > +++ b/lib/x86/desc.c > @@ -41,7 +41,8 @@ struct ex_record { > unsigned long handler; > }; > > -extern struct ex_record exception_table_start, exception_table_end; > +extern struct ex_record exception_table_start; > +extern long exception_table_size; > > static const char* exception_mnemonic(int vector) > { > @@ -108,12 +109,13 @@ static void check_exception_table(struct ex_regs > *regs) > { > struct ex_record *ex; > unsigned ex_val; > + int cnt = exception_table_size / sizeof(struct ex_record); > > ex_val = regs->vector | (regs->error_code << 16) | > (((regs->rflags >> 16) & 1) << 8); > asm("mov %0, %%gs:4" : : "r"(ex_val)); > > - for (ex = &exception_table_start; ex != &exception_table_end; ++ex) { > + for (ex = &exception_table_start; cnt-- > 0; ++ex) { > if (ex->rip == regs->rip) { > regs->rip = ex->handler; > return; > diff --git a/x86/flat.lds b/x86/flat.lds > index a278b56..108fad5 100644 > --- a/x86/flat.lds > +++ b/x86/flat.lds > @@ -9,6 +9,7 @@ SECTIONS > exception_table_start = .; > *(.data.ex) > exception_table_end = .; > + exception_table_size = exception_table_end - > exception_table_start; > } > . = ALIGN(16); > .rodata : { *(.rodata) } > > Thomas
diff --git a/lib/x86/desc.c b/lib/x86/desc.c index 451f504..cfc449f 100644 --- a/lib/x86/desc.c +++ b/lib/x86/desc.c @@ -41,7 +41,7 @@ struct ex_record { unsigned long handler; }; -extern struct ex_record exception_table_start, exception_table_end; +extern struct ex_record exception_table_start, *exception_table_end; static const char* exception_mnemonic(int vector) { @@ -113,7 +113,7 @@ static void check_exception_table(struct ex_regs *regs) (((regs->rflags >> 16) & 1) << 8); asm("mov %0, %%gs:4" : : "r"(ex_val)); - for (ex = &exception_table_start; ex != &exception_table_end; ++ex) { + for (ex = &exception_table_start; ex != (void*)&exception_table_end; ++ex) { if (ex->rip == regs->rip) { regs->rip = ex->handler; return;
Two global objects can't have the same address in C. Clang uses this fact to omit the check on the first iteration of the loop in check_exception_table. Signed-off-by: Bill Wendling <morbo@google.com> --- lib/x86/desc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)