Change in coreboot[master]: assert.h: Add a tag parameter to dead_code()

[Nico Huber (Code Review)]

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40140 )

Change subject: assert.h: Add a tag parameter to dead_code()
......................................................................

assert.h: Add a tag parameter to dead_code()

When dead_code() is used in inline functions in a header file, the
generated function names (based on the line number) may collide with
a dead_code() in the code file. Now that we are hit by such a case,
we need a quick solution: Add a tag argument for all invocations in
header files.

Change-Id: I0c548ce998cf8e28ae9f76b5c0ea5630b4e91ae2
Signed-off-by: Nico Huber 
---
M src/include/assert.h
M src/security/vboot/misc.h
2 files changed, 7 insertions(+), 7 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/40140/1

diff --git a/src/include/assert.h b/src/include/assert.h
index 990cee1..fbaf11a 100644
--- a/src/include/assert.h
+++ b/src/include/assert.h
@@ -65,13 +65,13 @@
  * ramstage/lib/bootmode.o: In function `display_init_required':
  * bootmode.c:42: undefined reference to `dead_code_assertion_failed_at_line_42'
  */
-#define __dead_code(line) do { \
-	extern void dead_code_assertion_failed_at_line_##line(void) \
+#define __dead_code(tag, line) do { \
+	extern void dead_code_assertion_failed##tag##_at_line_##line(void) \
    	__attribute__((noreturn)); \
-	dead_code_assertion_failed_at_line_##line(); \
+	dead_code_assertion_failed##tag##_at_line_##line(); \
 } while (0)
-#define _dead_code(line) __dead_code(line)
-#define dead_code() _dead_code(__LINE__)
+#define _dead_code(tag, line) __dead_code(tag, line)
+#define dead_code(tag) _dead_code(tag, __LINE__)

/* This can be used in the context of an expression of type 'type'. */
 #define dead_code_t(type) ({ \
diff --git a/src/security/vboot/misc.h b/src/security/vboot/misc.h
index fd422b2..22cc750 100644
--- a/src/security/vboot/misc.h
+++ b/src/security/vboot/misc.h
@@ -53,7 +53,7 @@
    else if (CONFIG(VBOOT_STARTS_IN_BOOTBLOCK))
    	return ENV_BOOTBLOCK;
    else
-		dead_code();
+		dead_code(_in_vboot_misc_h);
 }

static inline int verstage_should_load(void)
@@ -82,7 +82,7 @@
    	/* Post-RAM stages are "after the romstage" */
    	return !ENV_ROMSTAGE_OR_BEFORE;
    } else {
-		dead_code();
+		dead_code(_in_vboot_misc_h);
    }
 }

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40140 )

Change subject: assert.h: Add a tag parameter to dead_code()
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40140/1/src/include/assert.h 
File src/include/assert.h:

https://review.coreboot.org/c/coreboot/+/40140/1/src/include/assert.h@69 
PS1, Line 69: 	extern void dead_code_assertion_failed##tag##_at_line_##line(void) \
function definition argument 'void' should also have an identifier name

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40140 )

Change subject: assert.h: Add a tag parameter to dead_code()
......................................................................

Patch Set 1: Code-Review+1

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40140 )

Change subject: assert.h: Add a tag parameter to dead_code()
......................................................................

Patch Set 1: Code-Review+2

can't say that i really like this, but it helps working around the issue that currently breaks the tree

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40140 )

Change subject: assert.h: Add a tag parameter to dead_code()
......................................................................

Patch Set 1: Code-Review+2
> Patch Set 1: Code-Review+2
>
> can't say that i really like this, but it helps working around the issue that currently breaks the tree

Me neither

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40140 )

Change subject: assert.h: Add a tag parameter to dead_code()
......................................................................

Patch Set 1: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/40140/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40140/1//COMMIT_MSG@11 
PS1, Line 11: Now
Why are we hit now by this?

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40140 )

Change subject: assert.h: Add a tag parameter to dead_code()
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40140/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40140/1//COMMIT_MSG@11 
PS1, Line 11: Now
> Why are we hit now by this?

This happened because of the SPDX-replacing changes. After splitting the big CB:40045 change into smaller parts, one of the changes happened to move only part of the code using the macros, which resulted in two instances of `dead_code` in two different files to end up on the same line. And well, this broke master in a spectacular way...

I'm marking this as resolved so that ACR does not block submit. Since master is currently broken, this should get merged ASAP.

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40140 )

Change subject: assert.h: Add a tag parameter to dead_code()
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40140/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40140/1//COMMIT_MSG@11 
PS1, Line 11: Now
> Why are we hit now by this?

the line numbers changed due to the spdx license header conversion and that macro ended up being on the same line in two different compilation units that were linked together

[Felix Held (Code Review)]

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40140 )

Change subject: assert.h: Add a tag parameter to dead_code()
......................................................................

assert.h: Add a tag parameter to dead_code()

When dead_code() is used in inline functions in a header file, the
generated function names (based on the line number) may collide with
a dead_code() in the code file. Now that we are hit by such a case,
we need a quick solution: Add a tag argument for all invocations in
header files.

Change-Id: I0c548ce998cf8e28ae9f76b5c0ea5630b4e91ae2
Signed-off-by: Nico Huber 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40140
Tested-by: build bot (Jenkins) 
Reviewed-by: Felix Held 
Reviewed-by: Angel Pons 
Reviewed-by: Paul Menzel 
---
M src/include/assert.h
M src/security/vboot/misc.h
2 files changed, 7 insertions(+), 7 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Paul Menzel: Looks good to me, approved
  Felix Held: Looks good to me, approved
  Angel Pons: Looks good to me, approved

diff --git a/src/include/assert.h b/src/include/assert.h
index 990cee1..fbaf11a 100644
--- a/src/include/assert.h
+++ b/src/include/assert.h
@@ -65,13 +65,13 @@
  * ramstage/lib/bootmode.o: In function `display_init_required':
  * bootmode.c:42: undefined reference to `dead_code_assertion_failed_at_line_42'
  */
-#define __dead_code(line) do { \
-	extern void dead_code_assertion_failed_at_line_##line(void) \
+#define __dead_code(tag, line) do { \
+	extern void dead_code_assertion_failed##tag##_at_line_##line(void) \
    	__attribute__((noreturn)); \
-	dead_code_assertion_failed_at_line_##line(); \
+	dead_code_assertion_failed##tag##_at_line_##line(); \
 } while (0)
-#define _dead_code(line) __dead_code(line)
-#define dead_code() _dead_code(__LINE__)
+#define _dead_code(tag, line) __dead_code(tag, line)
+#define dead_code(tag) _dead_code(tag, __LINE__)

/* This can be used in the context of an expression of type 'type'. */
 #define dead_code_t(type) ({ \
diff --git a/src/security/vboot/misc.h b/src/security/vboot/misc.h
index fd422b2..22cc750 100644
--- a/src/security/vboot/misc.h
+++ b/src/security/vboot/misc.h
@@ -53,7 +53,7 @@
    else if (CONFIG(VBOOT_STARTS_IN_BOOTBLOCK))
    	return ENV_BOOTBLOCK;
    else
-		dead_code();
+		dead_code(_in_vboot_misc_h);
 }

static inline int verstage_should_load(void)
@@ -82,7 +82,7 @@
    	/* Post-RAM stages are "after the romstage" */
    	return !ENV_ROMSTAGE_OR_BEFORE;
    } else {
-		dead_code();
+		dead_code(_in_vboot_misc_h);
    }
 }

[9elements QA (Code Review)]

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40140 )

Change subject: assert.h: Add a tag parameter to dead_code()
......................................................................

Patch Set 2:

Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3
Emulation targets:
EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/2080
EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2079
EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2078

Please note: This test is under development and might not be accurate at all!