Use a status enum for reporting pass/fail

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 <[email protected]>

---
 lib/x86/desc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

lib/x86/desc.c +1 -1

index 451f504..cfc449f 100644
@@ -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)
 {

Quoted 26 lines · Bill Wendling

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 <[email protected]>

---
 lib/x86/desc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

lib/x86/desc.c +1 -1

index 451f504..cfc449f 100644
@@ -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));
 

lib/x86/desc.c static void check_exception_table(struct ex_regs *regs) · line 119

Quoted 6 lines

lib/x86/desc.c +1 -1

-    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:

lib/x86/desc.c +4 -2

index 451f504..0e9779b 100644
@@ -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;

x86/flat.lds +1 -0

index a278b56..108fad5 100644
@@ -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

Some values passed into "report" as "pass/fail" are larger than the size of the parameter. Instead use a status enum so that the size of the argument no longer matters.

Signed-off-by Bill Wendling <[email protected]>

Quoted 5 lines · Bill Wendling

Some values passed into "report" as "pass/fail" are larger than the size of the parameter. Instead use a status enum so that the size of the argument no longer matters.

Signed-off-by Bill Wendling <[email protected]>

The threading of these mails has me all kinds of confused. What is the relationship between all these patches? Did you perhaps intend to send some of these as v2?

[kvm-unit-tests PATCH 1/2] Use a status enum for reporting pass/fail [kvm-unit-tests PATCH 2/2] Use a status enum for reporting pass/fail [kvm-unit-tests PATCH 1/1] x86: use pointer for end of exception table [kvm-unit-tests PATCH 2/2] x86: use pointer for end of exception table

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. Avoid compariting inequality by using less-than.

Signed-off-by Bill Wendling <[email protected]>

---
 lib/x86/desc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

lib/x86/desc.c +0 -0

index 451f504..4002203 100644

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. Avoid compariting inequality by using less-than.

Signed-off-by Bill Wendling <[email protected]>

---
 lib/x86/desc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

lib/x86/desc.c +0 -0

index 451f504..4002203 100644

Some values passed into "report" as "pass/fail" are larger than the size of the parameter. Instead use a status enum so that the size of the argument no longer matters.

Signed-off-by Bill Wendling <[email protected]>

---
 lib/libcflat.h | 13 +++++++++++--
 lib/report.c   | 24 ++++++++++++------------
 2 files changed, 23 insertions(+), 14 deletions(-)

lib/libcflat.h +11 -2

index b6635d9..8f80a1c 100644
@@ -95,13 +95,22 @@ extern int vsnprintf(char *buf, int size, const char *fmt, va_list va)
 extern int vprintf(const char *fmt, va_list va)
 					__attribute__((format(printf, 1, 0)));
 
+enum status { PASSED, FAILED };
+
+#define STATUS(x) ((x) != 0 ? PASSED : FAILED)
+
+#define report(msg_fmt, status, ...) \
+	report_status(msg_fmt, STATUS(status) __VA_OPT__(,) __VA_ARGS__)
+#define report_xfail(msg_fmt, xfail, status, ...) \
+	report_xfail_status(msg_fmt, xfail, STATUS(status) __VA_OPT__(,) __VA_ARGS__)
+
 void report_prefix_pushf(const char *prefix_fmt, ...)
 					__attribute__((format(printf, 1, 2)));
 extern void report_prefix_push(const char *prefix);
 extern void report_prefix_pop(void);
-extern void report(const char *msg_fmt, unsigned pass, ...)
+extern void report_status(const char *msg_fmt, unsigned pass, ...)
 					__attribute__((format(printf, 1, 3)));
-extern void report_xfail(const char *msg_fmt, bool xfail, unsigned pass, ...)
+extern void report_xfail_status(const char *msg_fmt, bool xfail, enum status status, ...)
 					__attribute__((format(printf, 1, 4)));
 extern void report_abort(const char *msg_fmt, ...)
 					__attribute__((format(printf, 1, 2)))

lib/report.c +12 -12

index 2a5f549..4ba2ac0 100644
@@ -80,12 +80,12 @@ void report_prefix_pop(void)
 	spin_unlock(&lock);
 }
 
-static void va_report(const char *msg_fmt,
-		bool pass, bool xfail, bool skip, va_list va)
+static void va_report(const char *msg_fmt, enum status status, bool xfail,
+               bool skip, va_list va)
 {
 	const char *prefix = skip ? "SKIP"
-				  : xfail ? (pass ? "XPASS" : "XFAIL")
-					  : (pass ? "PASS"  : "FAIL");
+				  : xfail ? (status == PASSED ? "XPASS" : "XFAIL")
+					  : (status == PASSED ? "PASS"  : "FAIL");
 
 	spin_lock(&lock);
 

@@ -96,27 +96,27 @@ static void va_report(const char *msg_fmt,
 	puts("\n");
 	if (skip)
 		skipped++;
-	else if (xfail && !pass)
+	else if (xfail && status == FAILED)
 		xfailures++;
-	else if (xfail || !pass)
+	else if (xfail || status == FAILED)
 		failures++;
 
 	spin_unlock(&lock);
 }
 
-void report(const char *msg_fmt, unsigned pass, ...)
+void report_status(const char *msg_fmt, enum status status, ...)
 {
 	va_list va;
-	va_start(va, pass);
-	va_report(msg_fmt, pass, false, false, va);
+	va_start(va, status);
+	va_report(msg_fmt, status, false, false, va);
 	va_end(va);
 }
 
-void report_xfail(const char *msg_fmt, bool xfail, unsigned pass, ...)
+void report_xfail_status(const char *msg_fmt, bool xfail, enum status status, ...)
 {
 	va_list va;
-	va_start(va, pass);
-	va_report(msg_fmt, pass, xfail, false, va);
+	va_start(va, status);
+	va_report(msg_fmt, status, xfail, false, va);
 	va_end(va);
 }
 

Quoted 5 lines · Bill Wendling

Some values passed into "report" as "pass/fail" are larger than the size of the parameter. Instead use a status enum so that the size of the argument no longer matters.

Signed-off-by Bill Wendling <[email protected]>

I'm going to apply Thomas's argument reversal patch.

Note that the
commit message doesn't describe very well what is it that you're fixing (or rather, why it needs fixing).

Paolo

Quoted 92 lines

---
 lib/libcflat.h | 13 +++++++++++--
 lib/report.c   | 24 ++++++++++++------------
 2 files changed, 23 insertions(+), 14 deletions(-)

lib/libcflat.h +11 -2

index b6635d9..8f80a1c 100644
@@ -95,13 +95,22 @@ extern int vsnprintf(char *buf, int size, const char *fmt, va_list va)
 extern int vprintf(const char *fmt, va_list va)
                                      __attribute__((format(printf, 1, 0)));
 
+enum status { PASSED, FAILED };
+
+#define STATUS(x) ((x) != 0 ? PASSED : FAILED)
+
+#define report(msg_fmt, status, ...) \
+     report_status(msg_fmt, STATUS(status) __VA_OPT__(,) __VA_ARGS__)
+#define report_xfail(msg_fmt, xfail, status, ...) \
+     report_xfail_status(msg_fmt, xfail, STATUS(status) __VA_OPT__(,) __VA_ARGS__)
+
 void report_prefix_pushf(const char *prefix_fmt, ...)
                                      __attribute__((format(printf, 1, 2)));
 extern void report_prefix_push(const char *prefix);
 extern void report_prefix_pop(void);
-extern void report(const char *msg_fmt, unsigned pass, ...)
+extern void report_status(const char *msg_fmt, unsigned pass, ...)
                                      __attribute__((format(printf, 1, 3)));
-extern void report_xfail(const char *msg_fmt, bool xfail, unsigned pass, ...)
+extern void report_xfail_status(const char *msg_fmt, bool xfail, enum status status, ...)
                                      __attribute__((format(printf, 1, 4)));
 extern void report_abort(const char *msg_fmt, ...)
                                      __attribute__((format(printf, 1, 2)))

lib/report.c +12 -12

index 2a5f549..4ba2ac0 100644
@@ -80,12 +80,12 @@ void report_prefix_pop(void)
      spin_unlock(&lock);
 }
 
-static void va_report(const char *msg_fmt,
-             bool pass, bool xfail, bool skip, va_list va)
+static void va_report(const char *msg_fmt, enum status status, bool xfail,
+               bool skip, va_list va)
 {
      const char *prefix = skip ? "SKIP"
-                               : xfail ? (pass ? "XPASS" : "XFAIL")
-                                       : (pass ? "PASS"  : "FAIL");
+                               : xfail ? (status == PASSED ? "XPASS" : "XFAIL")
+                                       : (status == PASSED ? "PASS"  : "FAIL");
 
      spin_lock(&lock);
 

@@ -96,27 +96,27 @@ static void va_report(const char *msg_fmt,
      puts("\n");
      if (skip)
              skipped++;
-     else if (xfail && !pass)
+     else if (xfail && status == FAILED)
              xfailures++;
-     else if (xfail || !pass)
+     else if (xfail || status == FAILED)
              failures++;
 
      spin_unlock(&lock);
 }
 
-void report(const char *msg_fmt, unsigned pass, ...)
+void report_status(const char *msg_fmt, enum status status, ...)
 {
      va_list va;
-     va_start(va, pass);
-     va_report(msg_fmt, pass, false, false, va);
+     va_start(va, status);
+     va_report(msg_fmt, status, false, false, va);
      va_end(va);
 }
 
-void report_xfail(const char *msg_fmt, bool xfail, unsigned pass, ...)
+void report_xfail_status(const char *msg_fmt, bool xfail, enum status status, ...)
 {
      va_list va;
-     va_start(va, pass);
-     va_report(msg_fmt, pass, xfail, false, va);
+     va_start(va, status);
+     va_report(msg_fmt, status, xfail, false, va);
      va_end(va);
 }