Change in ...coreboot[master]: assert: Don't stringify 'message' in dead_code()

[Julius Werner (Code Review)]

Julius Werner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32113

Change subject: assert: Don't stringify 'message' in dead_code()
......................................................................

assert: Don't stringify 'message' in dead_code()

dead_code() is already supposed to be called with a string message, we
don't need to stringify the argument again (and doing so makes the
output look a bit weird).

Change-Id: I63399dc484e2150d8c027bc0256d9285e471f7cc
Signed-off-by: Julius Werner 
---
M src/include/assert.h
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/32113/1

diff --git a/src/include/assert.h b/src/include/assert.h
index afbed03..6a442d6 100644
--- a/src/include/assert.h
+++ b/src/include/assert.h
@@ -45,7 +45,7 @@
  * valid if a certain Kconfig option is set.
  */
 #define __dead_code(message, line) do { \
-	__attribute__((error(#message " in " __FILE__ ":" #line))) \
+	__attribute__((error(message " in " __FILE__ ":" #line))) \
    extern void dead_code_assertion_failed_##line(void); \
    dead_code_assertion_failed_##line(); \
 } while (0)

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32113 )

Change subject: assert: Don't stringify 'message' in dead_code()
......................................................................

Patch Set 1: Code-Review+2

Change in ...coreboot[master]: assert: Make dead_code() work at link-time instead of compile-time

[Julius Werner (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/32113

to look at the new patch set (#2).

Change subject: assert: Make dead_code() work at link-time instead of compile-time
......................................................................

assert: Make dead_code() work at link-time instead of compile-time

The dead_code() macro can be used to ensure that a certain code path is
compile-time eliminated (e.g. if you want to make sure it's never
executed for certain Kconfig combinations). Unfortunately, the current
implementation via __attribute__((error)) hits only at the GCC level.
This can catch code that can be compile-time eliminated based on state
within the same file, but it cannot be used in cases where a certain
library function is built but then garbage collected at link time.

This patch improves the macro by relying solely on the linker finding an
undefined reference. Unfortunately this makes the error message a little
less expressive (can no longer pass a custom string), but it is still
readable and one can add code comments next to the assertion to
elaborate further if necessary

Change-Id: I63399dc484e2150d8c027bc0256d9285e471f7cc
Signed-off-by: Julius Werner 
---
M src/include/assert.h
M src/mainboard/google/gru/board.h
2 files changed, 17 insertions(+), 13 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/32113/2

Change in ...coreboot[master]: assert: Make dead_code() work at link-time instead of compile-time

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32113 )

Change subject: assert: Make dead_code() work at link-time instead of compile-time
......................................................................

Patch Set 2: Code-Review+2

Change in ...coreboot[master]: assert: Make dead_code() work at link-time instead of compile-time

[Paul Menzel (Code Review)]

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

Change subject: assert: Make dead_code() work at link-time instead of compile-time
......................................................................

Patch Set 3: Code-Review+1

Change in ...coreboot[master]: assert: Make dead_code() work at link-time instead of compile-time

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32113 )

Change subject: assert: Make dead_code() work at link-time instead of compile-time
......................................................................

assert: Make dead_code() work at link-time instead of compile-time

The dead_code() macro can be used to ensure that a certain code path is
compile-time eliminated (e.g. if you want to make sure it's never
executed for certain Kconfig combinations). Unfortunately, the current
implementation via __attribute__((error)) hits only at the GCC level.
This can catch code that can be compile-time eliminated based on state
within the same file, but it cannot be used in cases where a certain
library function is built but then garbage collected at link time.

This patch improves the macro by relying solely on the linker finding an
undefined reference. Unfortunately this makes the error message a little
less expressive (can no longer pass a custom string), but it is still
readable and one can add code comments next to the assertion to
elaborate further if necessary

Change-Id: I63399dc484e2150d8c027bc0256d9285e471f7cc
Signed-off-by: Julius Werner 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32113
Reviewed-by: Paul Menzel 
Reviewed-by: Furquan Shaikh 
Tested-by: build bot (Jenkins) 
---
M src/include/assert.h
M src/mainboard/google/gru/board.h
2 files changed, 17 insertions(+), 13 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Paul Menzel: Looks good to me, but someone else must approve
  Furquan Shaikh: Looks good to me, approved

diff --git a/src/include/assert.h b/src/include/assert.h
index afbed03..60366352 100644
--- a/src/include/assert.h
+++ b/src/include/assert.h
@@ -43,18 +43,22 @@
  * will generate a compiler error even if the scope it was called from is dead
  * code. This may be useful to double-check things like constants that are only
  * valid if a certain Kconfig option is set.
+ *
+ * The error message when this hits will look like this:
+ *
+ * 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(message, line) do { \
-	__attribute__((error(#message " in " __FILE__ ":" #line))) \
-	extern void dead_code_assertion_failed_##line(void); \
-	dead_code_assertion_failed_##line(); \
+#define __dead_code(line) do { \
+	extern void dead_code_assertion_failed_at_line_##line(void); \
+	dead_code_assertion_failed_at_line_##line(); \
 } while (0)
-#define _dead_code(message, line) __dead_code(message, line)
-#define dead_code(message) _dead_code(message, __LINE__)
+#define _dead_code(line) __dead_code(line)
+#define dead_code() _dead_code(__LINE__)

/* This can be used in the context of an expression of type 'type'. */
-#define dead_code_t(type, message) ({ \
-	dead_code(message); \
+#define dead_code_t(type) ({ \
+	dead_code(); \
    *(type *)(uintptr_t)0; \
 })

diff --git a/src/mainboard/google/gru/board.h b/src/mainboard/google/gru/board.h
index e9545de..acf3fb9 100644
--- a/src/mainboard/google/gru/board.h
+++ b/src/mainboard/google/gru/board.h
@@ -29,16 +29,16 @@
 #define GPIO_BACKLIGHT	GPIO(4, C, 6)
 #define GPIO_EC_IN_RW	GPIO(0, A, 1)
 #define GPIO_EC_IRQ	GPIO(1, C, 2)
-#define GPIO_P15V_EN	dead_code_t(gpio_t, "PP1500 doesn't exist on scarlet.")
-#define GPIO_P18V_AUDIO_PWREN dead_code_t(gpio_t, "doesn't exist on scarlet.")
+#define GPIO_P15V_EN	dead_code_t(gpio_t)
+#define GPIO_P18V_AUDIO_PWREN dead_code_t(gpio_t)
 #define GPIO_P30V_EN	GPIO(0, B, 1)
 #define GPIO_SPK_PA_EN	GPIO(0, A, 2)
-#define GPIO_TP_RST_L	dead_code_t(gpio_t, "don't need TP_RST_L on scarlet.")
+#define GPIO_TP_RST_L	dead_code_t(gpio_t)
 #define GPIO_TPM_IRQ	GPIO(1, C, 1)
 #define GPIO_WP		GPIO(0, B, 5)
 #else
 #define GPIO_BL_EN	GPIO(1, C, 1)
-#define GPIO_BACKLIGHT	dead_code_t(gpio_t, "backlight controlled by ec")
+#define GPIO_BACKLIGHT	dead_code_t(gpio_t)
 #define GPIO_EC_IN_RW	GPIO(3, B, 0)
 #define GPIO_EC_IRQ	GPIO(0, A, 1)
 #define GPIO_P15V_EN	GPIO(0, B, 2)
@@ -53,7 +53,7 @@
 #if CONFIG(GRU_HAS_WLAN_RESET)
 #define GPIO_WLAN_RST_L	GPIO(1, B, 3)
 #else
-#define GPIO_WLAN_RST_L	dead_code_t(gpio_t, "no WLAN reset on this board in FW")
+#define GPIO_WLAN_RST_L	dead_code_t(gpio_t)
 #endif

void setup_chromeos_gpios(void);