Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33846
Change subject: Makefile.inc, payloads: Enable -Wvla ......................................................................
Makefile.inc, payloads: Enable -Wvla
Variable length arrays are dangerous, so let's make sure they don't sneak back into coreboot or any of the payloads.
Change-Id: Idf2488cf0efab51c9569a3789ae953368b61880c Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M Makefile.inc M payloads/bayou/Makefile M payloads/coreinfo/Makefile M payloads/libpayload/Makefile.inc M payloads/linuxcheck/Makefile M payloads/nvramcui/Makefile 6 files changed, 6 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/33846/1
diff --git a/Makefile.inc b/Makefile.inc index 362243e..498674e 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -401,7 +401,7 @@
CFLAGS_common += -pipe -g -nostdinc -std=gnu11 CFLAGS_common += -nostdlib -Wall -Wundef -Wstrict-prototypes -Wmissing-prototypes -CFLAGS_common += -Wwrite-strings -Wredundant-decls -Wno-trigraphs +CFLAGS_common += -Wwrite-strings -Wredundant-decls -Wno-trigraphs -Wvla CFLAGS_common += -Wstrict-aliasing -Wshadow -Wdate-time -Wtype-limits CFLAGS_common += -fno-common -ffreestanding -fno-builtin -fomit-frame-pointer CFLAGS_common += -ffunction-sections -fdata-sections -fno-pie diff --git a/payloads/bayou/Makefile b/payloads/bayou/Makefile index 7a4b08b..b893ca7 100644 --- a/payloads/bayou/Makefile +++ b/payloads/bayou/Makefile @@ -39,7 +39,7 @@ OBJECTS-$(CONFIG_NRV2B) += nrv2b.o OBJECTS-$(CONFIG_BUILTIN_LAR) += builtin-lar.o
-CFLAGS= -Wall -Werror -Os $(FFLAGS-y) +CFLAGS= -Wall -Wvla -Werror -Os $(FFLAGS-y) LDFLAGS=-Wl,-T,bayou.ldscript -static LIBGCC=$(shell $(CC) -m32 -print-libgcc-file-name)
diff --git a/payloads/coreinfo/Makefile b/payloads/coreinfo/Makefile index 50659d3..bcc5354 100644 --- a/payloads/coreinfo/Makefile +++ b/payloads/coreinfo/Makefile @@ -83,7 +83,7 @@ LPCC := CC="$(CC)" $(LIBPAYLOAD_OBJ)/bin/lpgcc LPAS := AS="$(AS)" $(LIBPAYLOAD_OBJ)/bin/lpas
-CFLAGS += -Wall -Werror -Os -fno-builtin $(CFLAGS_$(ARCH-y)) $(INCLUDES) +CFLAGS += -Wall -Wvla -Werror -Os -fno-builtin $(CFLAGS_$(ARCH-y)) $(INCLUDES)
ifneq ($(strip $(HAVE_DOTCONFIG)),) include $(src)/.config diff --git a/payloads/libpayload/Makefile.inc b/payloads/libpayload/Makefile.inc index 7787762..aae3921 100644 --- a/payloads/libpayload/Makefile.inc +++ b/payloads/libpayload/Makefile.inc @@ -63,7 +63,7 @@ CFLAGS += -nostdlib -fno-builtin -ffreestanding -fomit-frame-pointer CFLAGS += -ffunction-sections -fdata-sections CFLAGS += -Wall -Wundef -Wstrict-prototypes -Wmissing-prototypes -CFLAGS += -Wwrite-strings -Wredundant-decls -Wno-trigraphs +CFLAGS += -Wwrite-strings -Wredundant-decls -Wno-trigraphs -Wvla CFLAGS += -Wstrict-aliasing -Wshadow -Werror
$(obj)/libpayload-config.h: $(KCONFIG_AUTOHEADER) diff --git a/payloads/linuxcheck/Makefile b/payloads/linuxcheck/Makefile index b967369..838c90d 100644 --- a/payloads/linuxcheck/Makefile +++ b/payloads/linuxcheck/Makefile @@ -3,7 +3,7 @@ # build libpayload and put .config file in $(CURDIR) instead of ../libpayload # to avoid pollute the libpayload source directory and possible conflicts LPOPTS=obj="$(CURDIR)/build" DESTDIR="$(CURDIR)" DOTCONFIG="$(CURDIR)/.config" -CFLAGS += -Wall -Werror -Os -ffreestanding -nostdinc -nostdlib +CFLAGS += -Wall -Wvla -Werror -Os -ffreestanding -nostdinc -nostdlib ifeq ($(CONFIG_ARCH_X86),y) TARGETARCH = i386 endif diff --git a/payloads/nvramcui/Makefile b/payloads/nvramcui/Makefile index c6a9cd0..bf7053b 100644 --- a/payloads/nvramcui/Makefile +++ b/payloads/nvramcui/Makefile @@ -3,7 +3,7 @@ # build libpayload and put .config file in $(CURDIR) instead of ../libpayload # to avoid pollute the libpayload source directory and possible conflicts LPOPTS=obj="$(CURDIR)/build" DESTDIR="$(CURDIR)" DOTCONFIG="$(CURDIR)/.config" -CFLAGS += -Wall -Werror -Os -ffreestanding -nostdinc -nostdlib +CFLAGS += -Wall -Wvla -Werror -Os -ffreestanding -nostdinc -nostdlib
all: nvramcui.elf
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33846 )
Change subject: Makefile.inc, payloads: Enable -Wvla ......................................................................
Patch Set 3:
This change is ready for review.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33846 )
Change subject: Makefile.inc, payloads: Enable -Wvla ......................................................................
Patch Set 3: Code-Review+1
(1 comment)
Waiting on dependency chain
https://review.coreboot.org/#/c/33846/3/payloads/bayou/Makefile File payloads/bayou/Makefile:
PS3: This payload is not hooked up to the build system
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33846 )
Change subject: Makefile.inc, payloads: Enable -Wvla ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/33846/3/payloads/bayou/Makefile File payloads/bayou/Makefile:
PS3:
This payload is not hooked up to the build system
I'll just remove it for now then - I don't even know how to compile it locally
Hello Angel Pons, Julius Werner, David Hendricks, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33846
to look at the new patch set (#4).
Change subject: Makefile.inc, payloads: Enable -Wvla ......................................................................
Makefile.inc, payloads: Enable -Wvla
Variable length arrays are dangerous, so let's make sure they don't sneak back into coreboot or any of the payloads.
Change-Id: Idf2488cf0efab51c9569a3789ae953368b61880c Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M Makefile.inc M payloads/coreinfo/Makefile M payloads/libpayload/Makefile.inc M payloads/linuxcheck/Makefile M payloads/nvramcui/Makefile 5 files changed, 5 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/33846/4
Hello Angel Pons, Julius Werner, David Hendricks, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33846
to look at the new patch set (#5).
Change subject: Makefile.inc, payloads: Enable -Wvla ......................................................................
Makefile.inc, payloads: Enable -Wvla
Variable length arrays are dangerous, so let's make sure they don't sneak back into coreboot or any of the payloads.
Change-Id: Idf2488cf0efab51c9569a3789ae953368b61880c Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M Makefile.inc M payloads/coreinfo/Makefile M payloads/libpayload/Makefile.inc M payloads/linuxcheck/Makefile M payloads/nvramcui/Makefile 5 files changed, 5 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/33846/5
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33846 )
Change subject: Makefile.inc, payloads: Enable -Wvla ......................................................................
Patch Set 5: Code-Review+2
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33846 )
Change subject: Makefile.inc, payloads: Enable -Wvla ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33846/3/payloads/bayou/Makefile File payloads/bayou/Makefile:
PS3:
I'll just remove it for now then - I don't even know how to compile it locally
Done
Hello HAOUAS Elyes, Angel Pons, Julius Werner, David Hendricks, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33846
to look at the new patch set (#7).
Change subject: Makefile.inc, payloads: Enable -Wvla ......................................................................
Makefile.inc, payloads: Enable -Wvla
Variable length arrays are dangerous, so let's make sure they don't sneak back into coreboot or any of the payloads.
Change-Id: Idf2488cf0efab51c9569a3789ae953368b61880c Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M Makefile.inc M payloads/coreinfo/Makefile M payloads/libpayload/Makefile.inc M payloads/linuxcheck/Makefile M payloads/nvramcui/Makefile 5 files changed, 5 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/33846/7
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33846 )
Change subject: Makefile.inc, payloads: Enable -Wvla ......................................................................
Patch Set 7: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/33846/7/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/33846/7/Makefile.inc@404 PS7, Line 404: Wimplicit-fallthrough not related :) but it is ok if you add some thing to commit msg
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33846 )
Change subject: Makefile.inc, payloads: Enable -Wvla ......................................................................
Patch Set 7: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/33846/7/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/33846/7/Makefile.inc@404 PS7, Line 404: Wimplicit-fallthrough
not related :) […]
oops, sorry
Hello HAOUAS Elyes, Angel Pons, Julius Werner, David Hendricks, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33846
to look at the new patch set (#8).
Change subject: Makefile.inc, payloads: Enable -Wvla ......................................................................
Makefile.inc, payloads: Enable -Wvla
Variable length arrays are dangerous, so let's make sure they don't sneak back into coreboot or any of the payloads.
Change-Id: Idf2488cf0efab51c9569a3789ae953368b61880c Signed-off-by: Jacob Garber jgarber1@ualberta.ca --- M Makefile.inc M payloads/coreinfo/Makefile M payloads/libpayload/Makefile.inc M payloads/linuxcheck/Makefile M payloads/nvramcui/Makefile 5 files changed, 5 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/33846/8
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33846 )
Change subject: Makefile.inc, payloads: Enable -Wvla ......................................................................
Patch Set 11: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33846 )
Change subject: Makefile.inc, payloads: Enable -Wvla ......................................................................
Makefile.inc, payloads: Enable -Wvla
Variable length arrays are dangerous, so let's make sure they don't sneak back into coreboot or any of the payloads.
Change-Id: Idf2488cf0efab51c9569a3789ae953368b61880c Signed-off-by: Jacob Garber jgarber1@ualberta.ca Reviewed-on: https://review.coreboot.org/c/coreboot/+/33846 Reviewed-by: Patrick Georgi pgeorgi@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M Makefile.inc M payloads/coreinfo/Makefile M payloads/libpayload/Makefile.inc M payloads/linuxcheck/Makefile M payloads/nvramcui/Makefile 5 files changed, 5 insertions(+), 5 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved
diff --git a/Makefile.inc b/Makefile.inc index c275d1e..ffb22d3 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -402,7 +402,7 @@ CFLAGS_common += -pipe -g -nostdinc -std=gnu11 CFLAGS_common += -nostdlib -Wall -Wundef -Wstrict-prototypes -Wmissing-prototypes CFLAGS_common += -Wwrite-strings -Wredundant-decls -Wno-trigraphs -Wimplicit-fallthrough -CFLAGS_common += -Wstrict-aliasing -Wshadow -Wdate-time -Wtype-limits +CFLAGS_common += -Wstrict-aliasing -Wshadow -Wdate-time -Wtype-limits -Wvla CFLAGS_common += -fno-common -ffreestanding -fno-builtin -fomit-frame-pointer CFLAGS_common += -ffunction-sections -fdata-sections -fno-pie ifeq ($(CONFIG_COMPILER_GCC),y) diff --git a/payloads/coreinfo/Makefile b/payloads/coreinfo/Makefile index 1e9eb80..34c45d9 100644 --- a/payloads/coreinfo/Makefile +++ b/payloads/coreinfo/Makefile @@ -83,7 +83,7 @@ LPCC := CC="$(CC)" $(LIBPAYLOAD_OBJ)/bin/lpgcc LPAS := AS="$(AS)" $(LIBPAYLOAD_OBJ)/bin/lpas
-CFLAGS += -Wall -Wextra -Wmissing-prototypes -Werror +CFLAGS += -Wall -Wextra -Wmissing-prototypes -Wvla -Werror CFLAGS += -Os -fno-builtin $(CFLAGS_$(ARCH-y)) $(INCLUDES)
ifneq ($(strip $(HAVE_DOTCONFIG)),) diff --git a/payloads/libpayload/Makefile.inc b/payloads/libpayload/Makefile.inc index f0aaa27..4863d3f 100644 --- a/payloads/libpayload/Makefile.inc +++ b/payloads/libpayload/Makefile.inc @@ -62,7 +62,7 @@ CFLAGS += $(EXTRA_CFLAGS) $(INCLUDES) -Os -pipe -nostdinc -ggdb3 CFLAGS += -nostdlib -fno-builtin -ffreestanding -fomit-frame-pointer CFLAGS += -ffunction-sections -fdata-sections -CFLAGS += -Wall -Wundef -Wstrict-prototypes -Wmissing-prototypes +CFLAGS += -Wall -Wundef -Wstrict-prototypes -Wmissing-prototypes -Wvla CFLAGS += -Wwrite-strings -Wredundant-decls -Wno-trigraphs -Wimplicit-fallthrough CFLAGS += -Wstrict-aliasing -Wshadow -Werror
diff --git a/payloads/linuxcheck/Makefile b/payloads/linuxcheck/Makefile index b967369..838c90d 100644 --- a/payloads/linuxcheck/Makefile +++ b/payloads/linuxcheck/Makefile @@ -3,7 +3,7 @@ # build libpayload and put .config file in $(CURDIR) instead of ../libpayload # to avoid pollute the libpayload source directory and possible conflicts LPOPTS=obj="$(CURDIR)/build" DESTDIR="$(CURDIR)" DOTCONFIG="$(CURDIR)/.config" -CFLAGS += -Wall -Werror -Os -ffreestanding -nostdinc -nostdlib +CFLAGS += -Wall -Wvla -Werror -Os -ffreestanding -nostdinc -nostdlib ifeq ($(CONFIG_ARCH_X86),y) TARGETARCH = i386 endif diff --git a/payloads/nvramcui/Makefile b/payloads/nvramcui/Makefile index c6a9cd0..bf7053b 100644 --- a/payloads/nvramcui/Makefile +++ b/payloads/nvramcui/Makefile @@ -3,7 +3,7 @@ # build libpayload and put .config file in $(CURDIR) instead of ../libpayload # to avoid pollute the libpayload source directory and possible conflicts LPOPTS=obj="$(CURDIR)/build" DESTDIR="$(CURDIR)" DOTCONFIG="$(CURDIR)/.config" -CFLAGS += -Wall -Werror -Os -ffreestanding -nostdinc -nostdlib +CFLAGS += -Wall -Wvla -Werror -Os -ffreestanding -nostdinc -nostdlib
all: nvramcui.elf
-
Angel Pons (Code Review) -
HAOUAS Elyes (Code Review)
-
Jacob Garber (Code Review)
-
Patrick Georgi (Code Review)