| Message ID | 20170620110008.24413-1-leif.lindholm@linaro.org |
|---|---|
| State | Accepted |
| Commit | 2f93c5077fc30e319342df750846b1607fe5571e |
| Headers | show |
(+ Laszlo) On 20 June 2017 at 13:00, Leif Lindholm <leif.lindholm@linaro.org> wrote: > When building without LTO, gcc incorrectly resolves the hazards for > 'PciRegBase’ when inlining, leading to "may be used uninitialized" > warnings (and hence build failure with -Werror). > Eliminate this warning by explicitly initializing the variable to 0. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > > For those who have heard me mentioning this before but arguing against > upstreaming this patch: I only just tweaked that this warning doesn't > go away with more recent toolchains, but simply when switching to GCC5 > build profile, and hence LTO. Build failure still reproducible with > gcc 6.3.1 and GCC49. > /me annoyed We keep hitting this with GCC, and I profoundly dislike having to add redundant initialization sequences. Is there any other solution possible, e.g., disable this warning for certain builds? > ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c > index da93eb5829..18491c7378 100644 > --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c > +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c > @@ -343,6 +343,7 @@ ArmJunoSetNicMacAddress () > return Status; > } > > + PciRegBase = 0; > Status = InitPciDev (PciIo, &PciRegBase, &OldPciAttr); > if (EFI_ERROR (Status)) { > return Status; > -- > 2.11.0 >
On 20 June 2017 at 12:28, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > (+ Laszlo) > > On 20 June 2017 at 13:00, Leif Lindholm <leif.lindholm@linaro.org> wrote: >> When building without LTO, gcc incorrectly resolves the hazards for >> 'PciRegBase’ when inlining, leading to "may be used uninitialized" >> warnings (and hence build failure with -Werror). >> Eliminate this warning by explicitly initializing the variable to 0. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> > > Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Ryan Harkin <ryan.harkin@linaro.org> > >> --- >> >> For those who have heard me mentioning this before but arguing against >> upstreaming this patch: I only just tweaked that this warning doesn't >> go away with more recent toolchains, but simply when switching to GCC5 >> build profile, and hence LTO. Build failure still reproducible with >> gcc 6.3.1 and GCC49. >> > > /me annoyed > > We keep hitting this with GCC, and I profoundly dislike having to add > redundant initialization sequences. Is there any other solution > possible, e.g., disable this warning for certain builds? > >> ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c >> index da93eb5829..18491c7378 100644 >> --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c >> +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c >> @@ -343,6 +343,7 @@ ArmJunoSetNicMacAddress () >> return Status; >> } >> >> + PciRegBase = 0; >> Status = InitPciDev (PciIo, &PciRegBase, &OldPciAttr); >> if (EFI_ERROR (Status)) { >> return Status; >> -- >> 2.11.0 >>
On Tue, Jun 20, 2017 at 01:28:38PM +0200, Ard Biesheuvel wrote: > (+ Laszlo) > > On 20 June 2017 at 13:00, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > When building without LTO, gcc incorrectly resolves the hazards for > > 'PciRegBase’ when inlining, leading to "may be used uninitialized" > > warnings (and hence build failure with -Werror). > > Eliminate this warning by explicitly initializing the variable to 0. > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> > > Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Thanks! > > --- > > > > For those who have heard me mentioning this before but arguing against > > upstreaming this patch: I only just tweaked that this warning doesn't > > go away with more recent toolchains, but simply when switching to GCC5 > > build profile, and hence LTO. Build failure still reproducible with > > gcc 6.3.1 and GCC49. > > /me annoyed +1 > We keep hitting this with GCC, and I profoundly dislike having to add > redundant initialization sequences. Is there any other solution > possible, e.g., disable this warning for certain builds? I would really like to not disable it. When it gets it right, that's usually a pretty horrific bug caught. And if we switch to LTO as the norm, I guess these will become less noticeable over time. But it does bug me too. / Leif > > ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c > > index da93eb5829..18491c7378 100644 > > --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c > > +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c > > @@ -343,6 +343,7 @@ ArmJunoSetNicMacAddress () > > return Status; > > } > > > > + PciRegBase = 0; > > Status = InitPciDev (PciIo, &PciRegBase, &OldPciAttr); > > if (EFI_ERROR (Status)) { > > return Status; > > -- > > 2.11.0 > >
On Tue, Jun 20, 2017 at 12:44:09PM +0100, Ryan Harkin wrote: > On 20 June 2017 at 12:28, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > (+ Laszlo) > > > > On 20 June 2017 at 13:00, Leif Lindholm <leif.lindholm@linaro.org> wrote: > >> When building without LTO, gcc incorrectly resolves the hazards for > >> 'PciRegBase’ when inlining, leading to "may be used uninitialized" > >> warnings (and hence build failure with -Werror). > >> Eliminate this warning by explicitly initializing the variable to 0. > >> > >> Contributed-under: TianoCore Contribution Agreement 1.0 > >> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> > > > > Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Reviewed-by: Ryan Harkin <ryan.harkin@linaro.org> Thanks! Pushed as 2f93c5077f. > > > >> --- > >> > >> For those who have heard me mentioning this before but arguing against > >> upstreaming this patch: I only just tweaked that this warning doesn't > >> go away with more recent toolchains, but simply when switching to GCC5 > >> build profile, and hence LTO. Build failure still reproducible with > >> gcc 6.3.1 and GCC49. > >> > > > > /me annoyed > > > > We keep hitting this with GCC, and I profoundly dislike having to add > > redundant initialization sequences. Is there any other solution > > possible, e.g., disable this warning for certain builds? > > > >> ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c > >> index da93eb5829..18491c7378 100644 > >> --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c > >> +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c > >> @@ -343,6 +343,7 @@ ArmJunoSetNicMacAddress () > >> return Status; > >> } > >> > >> + PciRegBase = 0; > >> Status = InitPciDev (PciIo, &PciRegBase, &OldPciAttr); > >> if (EFI_ERROR (Status)) { > >> return Status; > >> -- > >> 2.11.0 > >>
On 06/20/17 14:00, Leif Lindholm wrote: > On Tue, Jun 20, 2017 at 01:28:38PM +0200, Ard Biesheuvel wrote: >> (+ Laszlo) >> >> On 20 June 2017 at 13:00, Leif Lindholm <leif.lindholm@linaro.org> wrote: >>> When building without LTO, gcc incorrectly resolves the hazards for >>> 'PciRegBase’ when inlining, leading to "may be used uninitialized" >>> warnings (and hence build failure with -Werror). >>> Eliminate this warning by explicitly initializing the variable to 0. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> >> >> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Thanks! > >>> --- >>> >>> For those who have heard me mentioning this before but arguing against >>> upstreaming this patch: I only just tweaked that this warning doesn't >>> go away with more recent toolchains, but simply when switching to GCC5 >>> build profile, and hence LTO. Build failure still reproducible with >>> gcc 6.3.1 and GCC49. >> >> /me annoyed > > +1 > >> We keep hitting this with GCC, and I profoundly dislike having to add >> redundant initialization sequences. Is there any other solution >> possible, e.g., disable this warning for certain builds? > > I would really like to not disable it. > When it gets it right, that's usually a pretty horrific bug caught. > And if we switch to LTO as the norm, I guess these will become less > noticeable over time. > But it does bug me too. Perhaps we should introduce two macros to "MdePkg/Include/Base.h", #define UNUSED_POINTER NULL #define UNUSED_INTEGER 0 These could be used to suppress such warnings, without the risk of misleading programmers (as to the real necessity of the variable assignment at hand). Thanks Laszlo
On 20 June 2017 at 17:44, Laszlo Ersek <lersek@redhat.com> wrote: > On 06/20/17 14:00, Leif Lindholm wrote: >> On Tue, Jun 20, 2017 at 01:28:38PM +0200, Ard Biesheuvel wrote: >>> (+ Laszlo) >>> >>> On 20 June 2017 at 13:00, Leif Lindholm <leif.lindholm@linaro.org> wrote: >>>> When building without LTO, gcc incorrectly resolves the hazards for >>>> 'PciRegBase’ when inlining, leading to "may be used uninitialized" >>>> warnings (and hence build failure with -Werror). >>>> Eliminate this warning by explicitly initializing the variable to 0. >>>> >>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> >>> >>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> Thanks! >> >>>> --- >>>> >>>> For those who have heard me mentioning this before but arguing against >>>> upstreaming this patch: I only just tweaked that this warning doesn't >>>> go away with more recent toolchains, but simply when switching to GCC5 >>>> build profile, and hence LTO. Build failure still reproducible with >>>> gcc 6.3.1 and GCC49. >>> >>> /me annoyed >> >> +1 >> >>> We keep hitting this with GCC, and I profoundly dislike having to add >>> redundant initialization sequences. Is there any other solution >>> possible, e.g., disable this warning for certain builds? >> >> I would really like to not disable it. >> When it gets it right, that's usually a pretty horrific bug caught. >> And if we switch to LTO as the norm, I guess these will become less >> noticeable over time. >> But it does bug me too. > > Perhaps we should introduce two macros to "MdePkg/Include/Base.h", > > #define UNUSED_POINTER NULL > #define UNUSED_INTEGER 0 > > These could be used to suppress such warnings, without the risk of > misleading programmers (as to the real necessity of the variable > assignment at hand). > Or perhaps #ifdef __GNUC__ #define USED(var) var = (typeof(var))0 #else #define USED(var) #endif
On 06/20/17 21:52, Ard Biesheuvel wrote: > On 20 June 2017 at 17:44, Laszlo Ersek <lersek@redhat.com> wrote: >> Perhaps we should introduce two macros to "MdePkg/Include/Base.h", >> >> #define UNUSED_POINTER NULL >> #define UNUSED_INTEGER 0 >> >> These could be used to suppress such warnings, without the risk of >> misleading programmers (as to the real necessity of the variable >> assignment at hand). >> > > Or perhaps > > #ifdef __GNUC__ > #define USED(var) var = (typeof(var))0 > #else > #define USED(var) > #endif > Sure, why not :) _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Tue, Jun 20, 2017 at 09:52:20PM +0200, Ard Biesheuvel wrote: > >>>> For those who have heard me mentioning this before but arguing against > >>>> upstreaming this patch: I only just tweaked that this warning doesn't > >>>> go away with more recent toolchains, but simply when switching to GCC5 > >>>> build profile, and hence LTO. Build failure still reproducible with > >>>> gcc 6.3.1 and GCC49. > >>> > >>> /me annoyed > >> > >> +1 > >> > >>> We keep hitting this with GCC, and I profoundly dislike having to add > >>> redundant initialization sequences. Is there any other solution > >>> possible, e.g., disable this warning for certain builds? > >> > >> I would really like to not disable it. > >> When it gets it right, that's usually a pretty horrific bug caught. > >> And if we switch to LTO as the norm, I guess these will become less > >> noticeable over time. > >> But it does bug me too. > > > > Perhaps we should introduce two macros to "MdePkg/Include/Base.h", > > > > #define UNUSED_POINTER NULL > > #define UNUSED_INTEGER 0 > > > > These could be used to suppress such warnings, without the risk of > > misleading programmers (as to the real necessity of the variable > > assignment at hand). > > > > Or perhaps > > #ifdef __GNUC__ > #define USED(var) var = (typeof(var))0 > #else > #define USED(var) > #endif So ... I prefer this version. However, semantically, for the situation here (may be used before initialized), "USED" does not make sense. Could we have an identical "INITIALIZED" instead? / Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c index da93eb5829..18491c7378 100644 --- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c +++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c @@ -343,6 +343,7 @@ ArmJunoSetNicMacAddress () return Status; } + PciRegBase = 0; Status = InitPciDev (PciIo, &PciRegBase, &OldPciAttr); if (EFI_ERROR (Status)) { return Status;
When building without LTO, gcc incorrectly resolves the hazards for 'PciRegBase’ when inlining, leading to "may be used uninitialized" warnings (and hence build failure with -Werror). Eliminate this warning by explicitly initializing the variable to 0. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> --- For those who have heard me mentioning this before but arguing against upstreaming this patch: I only just tweaked that this warning doesn't go away with more recent toolchains, but simply when switching to GCC5 build profile, and hence LTO. Build failure still reproducible with gcc 6.3.1 and GCC49. ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c | 1 + 1 file changed, 1 insertion(+)