This is an archive of the discontinued LLVM Phabricator instance.

[Sema] Do not emit -Wmissing-variable-declarations for register variables
ClosedPublic

Authored by nathanchance on Aug 8 2023, 1:22 PM.

Details

Summary

When building the Linux kernel with -Wmissing-variable-declarations,
there are several instances of warnings around variables declared with
register storage:

arch/x86/include/asm/asm.h:208:24: warning: no previous extern declaration for non-static variable 'current_stack_pointer' [-Wmissing-variable-declarations]
register unsigned long current_stack_pointer asm(_ASM_SP);
                       ^
arch/x86/include/asm/asm.h:208:10: note: declare 'static' if the variable is not intended to be used outside of this translation unit
register unsigned long current_stack_pointer asm(_ASM_SP);
         ^
1 warning generated.

The suggestion is invalid, as the variable cannot have both static and
register storage. Do not emit -Wmissing-variable-declarations for
register variables.

Closes: https://github.com/llvm/llvm-project/issues/64509
Link: https://lore.kernel.org/202308081050.sZEw4cQ5-lkp@intel.com/

Diff Detail

Event Timeline

nathanchance created this revision.Aug 8 2023, 1:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 8 2023, 1:22 PM
Herald added a subscriber: pengfei. · View Herald Transcript
nathanchance requested review of this revision.Aug 8 2023, 1:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 8 2023, 1:22 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
nickdesaulniers accepted this revision.Aug 8 2023, 1:25 PM

Thanks for the patch; I think it's better not to warn rather than expect users to add an
unsigned long currest_stack_pointer; declaration before the definition for register storage variables.

Maybe worth add a link to the GCC bug report in the commit message, too?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110947

clang/test/Sema/warn-missing-variable-declarations-register.c
3

@aaron.ballman do we need this REQUIRES clause?

This revision is now accepted and ready to land.Aug 8 2023, 1:25 PM
aaron.ballman accepted this revision.Aug 8 2023, 1:33 PM

LGTM aside from some minor nits.

clang/docs/ReleaseNotes.rst
126–127

Sphinx fixes

clang/test/Sema/warn-missing-variable-declarations-register.c
3

Nope, no need for REQUIRES here.

  • Update formatting in release notes
  • Add GCC bug link to commit message
  • Remove unnecessary REQUIRES: in test
This revision was landed with ongoing or failed builds.Aug 8 2023, 1:42 PM
This revision was automatically updated to reflect the committed changes.