Skip to content

Add -Wms-bitfield-padding warning when possible #139828

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ojhunt
Copy link
Contributor

@ojhunt ojhunt commented May 14, 2025

No description provided.

@ojhunt
Copy link
Contributor Author

ojhunt commented May 14, 2025

This is a draft PR as it might produce a bunch of noise, I've got a PR to fix the cases I can repeat locally: #139825

I suspect issues may occur in other configs and I'd like to hear from others before making this a non-draft PR

@ojhunt ojhunt requested review from dwblaikie and cor3ntin May 14, 2025 03:00
@ojhunt
Copy link
Contributor Author

ojhunt commented May 14, 2025

@mstorsjo @Endilll @IanWood1 given you're doing stage2+ builds would you folk mind seeing if this warning triggers for you? after applying #139825 if possible

@Endilll
Copy link
Contributor

Endilll commented May 14, 2025

@mstorsjo @Endilll @IanWood1 given you're doing stage2+ builds would you folk mind seeing if this warning triggers for you? after applying #139825 if possible

I applied changes from both PRs, and I'm seeing the following warnings (heavily deduplicated):

/home/user/endill/llvm-project/llvm/include/llvm/CodeGen/MachineInstr.h:152:11: warning: bit-field 'AsmPrinterFlags' of type 'uint8_t' (aka 'unsigned char') has a different storage size than the preceding bit-field (1 vs 4 bytes) and will not be packed under the Microsoft ABI [-Wms-bitfield-padding]
  152 |   uint8_t AsmPrinterFlags : LLVM_MI_ASMPRINTERFLAGS_BITS;
      |           ^
/home/user/endill/llvm-project/llvm/include/llvm/CodeGen/MachineInstr.h:147:12: note: preceding bit-field 'Flags' declared here with type 'uint32_t' (aka 'unsigned int')
  147 |   uint32_t Flags : LLVM_MI_FLAGS_BITS;
      |            ^

In file included from /home/user/endill/llvm-project/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:14:
/home/user/endill/llvm-project/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h:66:14: warning: bit-field 'IsSubfield' of type 'uint16_t' (aka 'unsigned short') has a different storage size than the preceding bit-field (2 vs 4 bytes) and will not be packed under the Microsoft ABI [-Wms-bitfield-padding]
   66 |     uint16_t IsSubfield : 1;
      |              ^
/home/user/endill/llvm-project/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h:63:9: note: preceding bit-field 'DataOffset' declared here with type 'int'
   63 |     int DataOffset : 31;
      |         ^

/home/user/endill/llvm-project/clang/lib/Basic/DiagnosticIDs.cpp:90:12: warning: bit-field 'OptionGroupIndex' of type 'uint16_t' (aka 'unsigned short') has a different storage size than the preceding bit-field (2 vs 1 bytes) and will not be packed under the Microsoft ABI [-Wms-bitfield-padding]
   90 |   uint16_t OptionGroupIndex : 15;
      |            ^
/home/user/endill/llvm-project/clang/lib/Basic/DiagnosticIDs.cpp:88:11: note: preceding bit-field 'WarnShowInSystemMacro' declared here with type 'uint8_t' (aka 'unsigned char')
   88 |   uint8_t WarnShowInSystemMacro : 1;
      |           ^

/home/user/endill/llvm-project/clang/include/clang/AST/DeclTemplate.h:1862:8: warning: bit-field 'StrictPackMatch' of type 'bool' has a different storage size than the preceding bit-field (1 vs 4 bytes) and will not be packed under the Microsoft ABI [-Wms-bitfield-padding]
 1862 |   bool StrictPackMatch : 1;
      |        ^
/home/user/endill/llvm-project/clang/include/clang/AST/DeclTemplate.h:1855:12: note: preceding bit-field 'SpecializationKind' declared here with type 'unsigned int'
 1855 |   unsigned SpecializationKind : 3;
      |            ^

/home/user/endill/llvm-project/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h:1667:8: warning: bit-field 'Visited' of type 'bool' has a different storage size than the preceding bit-field (1 vs 4 bytes) and will not be packed under the Microsoft ABI [-Wms-bitfield-padding]
 1667 |   bool Visited : 1;
      |        ^
/home/user/endill/llvm-project/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h:1664:12: note: preceding bit-field 'BlockID' declared here with type 'unsigned int'
 1664 |   unsigned BlockID : 31;
      |            ^

/home/user/endill/llvm-project/clang/include/clang/Sema/ScopeInfo.h:119:8: warning: bit-field 'HasBranchProtectedScope' of type 'bool' has a different storage size than the preceding bit-field (1 vs 4 bytes) and will not be packed under the Microsoft ABI [-Wms-bitfield-padding]
  119 |   bool HasBranchProtectedScope : 1;
      |        ^
/home/user/endill/llvm-project/clang/include/clang/Sema/ScopeInfo.h:115:13: note: preceding bit-field 'Kind' declared here with type 'ScopeKind'
  115 |   ScopeKind Kind : 3;
      |             ^

/home/user/endill/llvm-project/clang/include/clang/Sema/Overload.h:987:14: warning: bit-field 'IsADLCandidate' of type 'unsigned int' has a different storage size than the preceding bit-field (4 vs 1 bytes) and will not be packed under the Microsoft ABI [-Wms-bitfield-padding]
  987 |     unsigned IsADLCandidate : 1;
      |              ^
/home/user/endill/llvm-project/clang/include/clang/Sema/Overload.h:983:10: note: preceding bit-field 'StrictPackMatch' declared here with type 'bool'
  983 |     bool StrictPackMatch : 1;
      |          ^
      
/home/user/endill/llvm-project/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:272:8: warning: bit-field 'AnalyzerWerror' of type 'bool' has a different storage size than the preceding bit-field (1 vs 4 bytes) and will not be packed under the Microsoft ABI [-Wms-bitfield-padding]
  272 |   bool AnalyzerWerror : 1;
      |        ^
/home/user/endill/llvm-project/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:269:12: note: preceding bit-field 'NoRetryExhausted' declared here with type 'unsigned int'
  269 |   unsigned NoRetryExhausted : 1;
      |            ^

Most of them are in headers, so you should take care of them before people's build logs are flooded.

P.S. I'm not really doing 2-stage builds. What I do is that I use nightly builds from https://apt.llvm.org, and update 1-2 times per month on average. TIL that the second stage of 2-stage builds is not compatible with distcc, which makes sense, so this was a somewhat painful exercise.

@ojhunt
Copy link
Contributor Author

ojhunt commented May 14, 2025

@mstorsjo @Endilll @IanWood1 given you're doing stage2+ builds would you folk mind seeing if this warning triggers for you? after applying #139825 if possible

I applied changes from both PRs, and I'm seeing the following warnings (heavily deduplicated):

Thank you very much! I'll try to add the fixes for those ones.

What targets do you have enabled?

@Endilll
Copy link
Contributor

Endilll commented May 14, 2025

@mstorsjo @Endilll @IanWood1 given you're doing stage2+ builds would you folk mind seeing if this warning triggers for you? after applying #139825 if possible

I applied changes from both PRs, and I'm seeing the following warnings (heavily deduplicated):

Thank you very much! I'll try to add the fixes for those ones.

What targets do you have enabled?

Only X86

@ojhunt
Copy link
Contributor Author

ojhunt commented May 16, 2025

@mstorsjo @Endilll @IanWood1 given you're doing stage2+ builds would you folk mind seeing if this warning triggers for you? after applying #139825 if possible

I applied changes from both PRs, and I'm seeing the following warnings (heavily deduplicated):

Thank you very much! I'll try to add the fixes for those ones.
What targets do you have enabled?

Only X86

would you mind checking now that #139825 has been merged?

@Endilll
Copy link
Contributor

Endilll commented May 16, 2025

I'll do a 2-stage build again, now with all the targets enabled. Should I enable any additional diagnostic flags for the second stage?

@ojhunt
Copy link
Contributor Author

ojhunt commented May 16, 2025

I'll do a 2-stage build again, now with all the targets enabled. Should I enable any additional diagnostic flags for the second stage?

maybe -Werror=ms-bitfield-padding so it dies immediately?

I think I got everything, but I was dealing with some slightly irksome lbc++ linker problems

@Endilll
Copy link
Contributor

Endilll commented May 16, 2025

In addition to all targets, I also built clang-tools-extra, and there's a warning there:

/home/user/endill/llvm-project/clang-tools-extra/clangd/FuzzyMatch.h:128:12: warning: bit-field 'Prev' of type 'Action' (aka 'bool') has a different storage size than the preceding bit-field (1 vs 4 bytes) and will not be packed under the Microsoft ABI [-Wms-bitfield-padding]
  128 |     Action Prev : 1;
      |            ^
/home/user/endill/llvm-project/clang-tools-extra/clangd/FuzzyMatch.h:127:16: note: preceding bit-field 'Score' declared here with type 'int'
  127 |     signed int Score : 15;
      |                ^

@ojhunt
Copy link
Contributor Author

ojhunt commented May 16, 2025

In addition to all targets, I also built clang-tools-extra, and there's a warning there:

/home/user/endill/llvm-project/clang-tools-extra/clangd/FuzzyMatch.h:128:12: warning: bit-field 'Prev' of type 'Action' (aka 'bool') has a different storage size than the preceding bit-field (1 vs 4 bytes) and will not be packed under the Microsoft ABI [-Wms-bitfield-padding]
  128 |     Action Prev : 1;
      |            ^
/home/user/endill/llvm-project/clang-tools-extra/clangd/FuzzyMatch.h:127:16: note: preceding bit-field 'Score' declared here with type 'int'
  127 |     signed int Score : 15;
      |                ^

ah ha, thank you!

@Endilll
Copy link
Contributor

Endilll commented May 16, 2025

I figured out I should compile lld and lldb, too. Here are additional warnings:

/home/user/endill/llvm-project/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h:192:12: warning: bit-field 'm_sibling_idx' of type 'uint32_t' (aka 'unsigned int') has a different storage size than the preceding bit-field (4 vs 8 bytes) and will not be packed under the Microsoft ABI [-Wms-bitfield-padding]
  192 |   uint32_t m_sibling_idx : 31, m_has_children : 1;
      |            ^
/home/user/endill/llvm-project/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h:187:15: note: preceding bit-field 'm_parent_idx' declared here with type 'dw_offset_t' (aka 'unsigned long')
  187 |   dw_offset_t m_parent_idx : 64 - DW_DIE_OFFSET_MAX_BITSIZE;
      |               ^
      
/home/user/endill/llvm-project/lldb/source/Plugins/Language/ObjC/NSArray.cpp:104:16: warning: bit-field '_priv1' of type 'uint64_t' (aka 'unsigned long') has a different storage size than the preceding bit-field (8 vs 4 bytes) and will not be packed under the Microsoft ABI [-Wms-bitfield-padding]
  104 |       uint64_t _priv1 : 4;
      |                ^
/home/user/endill/llvm-project/lldb/source/Plugins/Language/ObjC/NSArray.cpp:103:16: note: preceding bit-field '_size' declared here with type 'uint32_t' (aka 'unsigned int')
  103 |       uint32_t _size : 28;
      |                ^
      
/home/user/endill/llvm-project/lldb/source/Plugins/Language/ObjC/NSSet.cpp:65:14: warning: bit-field '_szidx' of type 'uint32_t' (aka 'unsigned int') has a different storage size than the preceding bit-field (4 vs 8 bytes) and will not be packed under the Microsoft ABI [-Wms-bitfield-padding]
   65 |     uint32_t _szidx : 6;
      |              ^
/home/user/endill/llvm-project/lldb/source/Plugins/Language/ObjC/NSSet.cpp:64:14: note: preceding bit-field '_used' declared here with type 'uint64_t' (aka 'unsigned long')
   64 |     uint64_t _used : 58;
      |              ^
      
/home/user/endill/llvm-project/lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:276:16: warning: bit-field '_kvo' of type 'uint32_t' (aka 'unsigned int') has a different storage size than the preceding bit-field (4 vs 8 bytes) and will not be packed under the Microsoft ABI [-Wms-bitfield-padding]
  276 |       uint32_t _kvo : 1;
      |                ^
/home/user/endill/llvm-project/lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:275:16: note: preceding bit-field '_used' declared here with type 'uint64_t' (aka 'unsigned long')
  275 |       uint64_t _used : 58;
      |                ^
      
/home/user/endill/llvm-project/lld/wasm/InputChunks.h:107:12: warning: bit-field 'live' of type 'unsigned int' has a different storage size than the preceding bit-field (4 vs 1 bytes) and will not be packed under the Microsoft ABI [-Wms-bitfield-padding]
  107 |   unsigned live : 1;
      |            ^
/home/user/endill/llvm-project/lld/wasm/InputChunks.h:102:11: note: preceding bit-field 'sectionKind' declared here with type 'uint8_t' (aka 'unsigned char')
  102 |   uint8_t sectionKind : 3;
      |           ^

@ojhunt
Copy link
Contributor Author

ojhunt commented May 16, 2025

ah ha, lets see if I can convince my local build to just build all the things

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants