public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Mike Beaton" <mjsbeaton@gmail.com>
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <ardb@google.com>
Subject: [edk2-devel] RELEASE CLANGPDB OVMF currently does not compile
Date: Sun, 10 Dec 2023 00:25:47 +0000	[thread overview]
Message-ID: <CAHzAAWQBdTXzq7KnTyPVWoE5gtKe+O7Jxe69K_TC8UphDar8vA@mail.gmail.com> (raw)

Dear Ard,

Thanks for your attention on my other issue, about NOOPT CLANGDWARF OVMF.

This one is about RELEASE CLANGPDB OVMF, which currently does not
compile, giving:

```
/home/mjsbeaton/OpenSource/edk2/OvmfPkg/VirtioSerialDxe/VirtioSerial.c:28:22:
error: variable 'EventNames' is not needed and will not be emitted
[-Werror,-Wunneeded-internal-declaration]
STATIC CONST CHAR8  *EventNames[] = {
                     ^
1 error generated.
```

I logged this at https://bugzilla.tianocore.org/show_bug.cgi?id=4620

This _can_ be fixed by:

```
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -1754,7 +1754,7 @@ DEFINE CLANGPDB_X64_PREFIX           = ENV(CLANG_BIN)
 DEFINE CLANGPDB_IA32_TARGET          = -target i686-unknown-windows-gnu
 DEFINE CLANGPDB_X64_TARGET           = -target x86_64-unknown-windows-gnu

-DEFINE CLANGPDB_WARNING_OVERRIDES    = -Wno-parentheses-equality
-Wno-tautological-compare
-Wno-tautological-constant-out-of-range-compare -Wno-empty-body
-Wno-unused-const-variable -Wno-varargs -Wno-unknown-warning-option
-Wno-unused-but-set-variable -Wno-unused-const-variable
-Wno-unaligned-access -Wno-microsoft-enum-forward-reference
+DEFINE CLANGPDB_WARNING_OVERRIDES    = -Wno-parentheses-equality
-Wno-tautological-compare
-Wno-tautological-constant-out-of-range-compare -Wno-empty-body
-Wno-unused-const-variable -Wno-varargs -Wno-unknown-warning-option
-Wno-unused-but-set-variable -Wno-unused-const-variable
-Wno-unaligned-access -Wno-unneeded-internal-declaration
-Wno-microsoft-enum-forward-reference
 DEFINE CLANGPDB_ALL_CC_FLAGS         = DEF(GCC48_ALL_CC_FLAGS)
DEF(CLANGPDB_WARNING_OVERRIDES) -fno-stack-protector -funsigned-char
-ftrap-function=undefined_behavior_has_been_optimized_away_by_clang
-Wno-address -Wno-shift-negative-value -Wno-unknown-pragmas
-Wno-incompatible-library-redeclaration -Wno-null-dereference
-mno-implicit-float -mms-bitfields -mno-stack-arg-probe -nostdlib
-nostdlibinc -fseh-exceptions

 ###########################
```

which duplicates https://github.com/tianocore/edk2/commit/d3225577123,
which already applied the same change to CLANGDWARF.

That change was discussed and approved here:
https://edk2.groups.io/g/devel/topic/98807494 - however, I'd like to
belatedly object, if I may!

There is currently exactly one line of code which needs this warning
to be disabled (at least, in OVMF), the line mentioned above. So if we
were going to bring the warning back, now rather than later would be
the time to do it. The issue at that line can, of course, be worked
around by removing the STATIC (and presumably adding a comment, so
that someone doesn't mistakenly add it back again).

I would guess that there must be several other places where people
_have_ already tiptoed round this issue before, in EDK-2 code, though
a quick search for the warning name itself only throws up one such
instance in OpenSslLib.

The advantage of tiptoeing round the issue (in the reasonably rare
cases where needed) instead of disabling the error check, is that the
check may well show up genuine issues in code (perhaps most obviously,
variables left unused after code changes).

For that reason, I'd propose re-enabling the warning in CLANGDWARF,
and removing the STATIC (and adding a comment) in the relevant line in
VirtioSerialDxe.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112255): https://edk2.groups.io/g/devel/message/112255
Mute This Topic: https://groups.io/mt/103083030/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



             reply	other threads:[~2023-12-10  0:26 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-10  0:25 Mike Beaton [this message]
2023-12-10  8:07 ` [edk2-devel] RELEASE CLANGPDB OVMF currently does not compile Ard Biesheuvel
2023-12-10  9:41   ` Mike Beaton
2023-12-10 10:20     ` Mike Beaton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHzAAWQBdTXzq7KnTyPVWoE5gtKe+O7Jxe69K_TC8UphDar8vA@mail.gmail.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox