[NFC] Address more bit-field storage sizes #140493
Conversation
|
@llvm/pr-subscribers-lld @llvm/pr-subscribers-lld-wasm Author: Oliver Hunt (ojhunt) ChangesFollow on work from #139825 Full diff: https://github.com/llvm/llvm-project/pull/140493.diff 6 Files Affected:
diff --git a/lld/wasm/InputChunks.h b/lld/wasm/InputChunks.h
index 1fe78d76631f1..4bae235b247c5 100644
--- a/lld/wasm/InputChunks.h
+++ b/lld/wasm/InputChunks.h
@@ -99,15 +99,15 @@ class InputChunk {
// the beginning of the output section this chunk was assigned to.
int32_t outSecOff = 0;
- uint8_t sectionKind : 3;
+ uint32_t sectionKind : 3;
// Signals that the section is part of the output. The garbage collector,
// and COMDAT handling can set a sections' Live bit.
// If GC is disabled, all sections start out as live by default.
- unsigned live : 1;
+ uint32_t live : 1;
// Signals the chunk was discarded by COMDAT handling.
- unsigned discarded : 1;
+ uint32_t discarded : 1;
protected:
InputChunk(ObjFile *f, Kind k, StringRef name, uint32_t alignment = 0,
diff --git a/lldb/source/Plugins/Language/ObjC/NSArray.cpp b/lldb/source/Plugins/Language/ObjC/NSArray.cpp
index 25376e064879d..444286692994d 100644
--- a/lldb/source/Plugins/Language/ObjC/NSArray.cpp
+++ b/lldb/source/Plugins/Language/ObjC/NSArray.cpp
@@ -101,7 +101,7 @@ namespace Foundation1010 {
uint32_t _used;
uint32_t _offset;
uint32_t _size : 28;
- uint64_t _priv1 : 4;
+ uint32_t _priv1 : 4;
uint32_t _priv2;
uint32_t _data;
};
diff --git a/lldb/source/Plugins/Language/ObjC/NSDictionary.cpp b/lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
index ef1c2c89fe125..4f5c54b2c077c 100644
--- a/lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
+++ b/lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
@@ -119,7 +119,7 @@ class NSDictionaryISyntheticFrontEnd : public SyntheticChildrenFrontEnd {
struct DataDescriptor_64 {
uint64_t _used : 58;
- uint32_t _szidx : 6;
+ uint64_t _szidx : 6;
};
struct DictionaryItemDescriptor {
@@ -273,7 +273,7 @@ namespace Foundation1100 {
struct DataDescriptor_64 {
uint64_t _used : 58;
- uint32_t _kvo : 1;
+ uint64_t _kvo : 1;
uint64_t _size;
uint64_t _mutations;
uint64_t _objs_addr;
@@ -308,7 +308,7 @@ namespace Foundation1428 {
struct DataDescriptor_64 {
uint64_t _used : 58;
- uint32_t _kvo : 1;
+ uint64_t _kvo : 1;
uint64_t _size;
uint64_t _buffer;
uint64_t GetSize() { return _size; }
diff --git a/lldb/source/Plugins/Language/ObjC/NSSet.cpp b/lldb/source/Plugins/Language/ObjC/NSSet.cpp
index 7d814e656dc5f..bee6d5ceb41b6 100644
--- a/lldb/source/Plugins/Language/ObjC/NSSet.cpp
+++ b/lldb/source/Plugins/Language/ObjC/NSSet.cpp
@@ -62,7 +62,7 @@ class NSSetISyntheticFrontEnd : public SyntheticChildrenFrontEnd {
struct DataDescriptor_64 {
uint64_t _used : 58;
- uint32_t _szidx : 6;
+ uint64_t _szidx : 6;
};
struct SetItemDescriptor {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
index 72aeb2743b1e2..022a64e3d7f6f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -189,7 +189,7 @@ class DWARFDebugInfoEntry {
// If it is zero, then the DIE doesn't have children,
// or the DWARF claimed it had children but the DIE
// only contained a single NULL terminating child.
- uint32_t m_sibling_idx : 31, m_has_children : 1;
+ uint64_t m_sibling_idx : 31, m_has_children : 1;
uint16_t m_abbr_idx = 0;
/// A copy of the DW_TAG value so we don't have to go through the compile
/// unit abbrev table
diff --git a/llvm/include/llvm/Demangle/ItaniumDemangle.h b/llvm/include/llvm/Demangle/ItaniumDemangle.h
index f4569850b093c..8041d5447095c 100644
--- a/llvm/include/llvm/Demangle/ItaniumDemangle.h
+++ b/llvm/include/llvm/Demangle/ItaniumDemangle.h
@@ -19,6 +19,7 @@
#include "DemangleConfig.h"
#include "StringViewExtras.h"
#include "Utility.h"
+#include "llvm/Support/Compiler.h"
#include <algorithm>
#include <cctype>
#include <cstdint>
|
ojhunt
force-pushed
the
users/ojhunt/more-bitfield-size-fixes
branch
from
6234ea2 to
90e4c9d
Compare
| @@ -99,15 +99,15 @@ class InputChunk { | |||
| // the beginning of the output section this chunk was assigned to. | |||
| int32_t outSecOff = 0; | |||
|
|
|||
| uint8_t sectionKind : 3; | |||
| uint32_t sectionKind : 3; | |||
There was a problem hiding this comment.
Can you explain why uint32_t is better than uint8_t here?
There was a problem hiding this comment.
(Perhaps some background in the PR description would be useful).
There was a problem hiding this comment.
Unfortunately I can :(
The MS ABI only packs bit-fields if they have the same storage size, e.g.
struct S {
bool b: 1;
int i: 1
};is 8 bytes.
It's a large part of why we use preferred_type and unsigned bit-fields instead of just enum bit-fields.
This PR is the continuing series of fixing the existing cases where the storage types are wrong (per MS :D), so we can then enable the warning for this behavior, so that we can then just start using enums without risking silently terrible padding with MSVC
There was a problem hiding this comment.
By MS ABI I assume you mean mircrosoft?
I guess this is not actually a regression on non-MS compilers since the bit width means that type does not actually impact the size in the struct?
There was a problem hiding this comment.
Yes, let me check whether I'm meant to be using Microsoft or MS (I may have gotten conflated with when I should use MSVC).
This does not change behavior with anything other than the MS abi. When building with the MS abi this changes the field padding to match clang/gcc/every other ABI.
The long term reason for the change is so we can enable -Wms-bitfield-padding without immediately overwhelming stage2 or trunk builders with the new padding warning.
Once that's enabled we can start moving to correctly typed enum bit-fields with some confidence we aren't going to regress layout under the ms abi.
Follow on work from #139825. As previously this is fixing a few more cases where adjacent bit-fields have types with different base storage sizes, as the MS ABI will not pack those bit-fields.
ojhunt
force-pushed
the
users/ojhunt/more-bitfield-size-fixes
branch
from
90e4c9d to
7bf3eba
Compare
|
Force push with no PR changes other than attaching more information to the commit message. |
Follow on work from #139825.
As previously this is fixing a few more cases where adjacent bit-fields
have types with different base storage sizes, as the MS ABI will not
pack those bit-fields.