[PATCH] D70606: LLD: CET shadow stack support on Windows

[Rui Ueyama via Phabricator via llvm-commits]

ruiu accepted this revision.
ruiu added a comment.
This revision is now accepted and ready to land.

LGTM with these fixes.



================
Comment at: lld/COFF/Writer.cpp:172
+public:
+  ExtendedDllCharacteristicsChunk(unsigned C)
+      : Characteristics(C) {}
----------------
The constructor should take uint32 instead of unsigned.


================
Comment at: lld/COFF/Writer.cpp:173
+  ExtendedDllCharacteristicsChunk(unsigned C)
+      : Characteristics(C) {}
+
----------------
C -> c


================
Comment at: lld/COFF/Writer.cpp:175-177
+  size_t getSize() const override {
+    return sizeof(Characteristics);
+  }
----------------
Let's just return 4.


================
Comment at: lld/COFF/Writer.cpp:180
+  void writeTo(uint8_t *B) const override {
+    memcpy(B, &Characteristics, sizeof(ExtendedDLLCharacteristics));
+  }
----------------
Likewise, I'd replace sizeof() with 4.


================
Comment at: lld/COFF/Writer.cpp:180
+  void writeTo(uint8_t *B) const override {
+    memcpy(B, &Characteristics, sizeof(ExtendedDLLCharacteristics));
+  }
----------------
ruiu wrote:
> Likewise, I'd replace sizeof() with 4.
B -> buf


================
Comment at: lld/COFF/Writer.cpp:183
+
+  uint32_t Characteristics = 0;
+};
----------------
Characteristics -> characteristics


================
Comment at: lld/COFF/Writer.cpp:957
+  if (config->cetCompat) {
+    ExtendedDllCharacteristicsChunk *extendedDllChars = make<ExtendedDllCharacteristicsChunk>(IMAGE_DLL_CHARACTERISTICS_EX_CET_COMPAT);
+    debugRecords.push_back({COFF::IMAGE_DEBUG_TYPE_EX_DLLCHARACTERISTICS, extendedDllChars});
----------------
Break this line so that it fits within 80 columns.


================
Comment at: llvm/tools/llvm-readobj/COFFDumper.cpp:751
+      if (D.Type == COFF::IMAGE_DEBUG_TYPE_EX_DLLCHARACTERISTICS) {
+        // FIXME right now the only possible value would fit in 8 bits, but that might change in the future
+        uint16_t Characteristics = RawData[0];
----------------
80 cols



================
Comment at: llvm/tools/llvm-readobj/COFFDumper.cpp:753
+        uint16_t Characteristics = RawData[0];
+        W.printFlags ("ExtendedCharacteristics", Characteristics, makeArrayRef(PEExtendedDLLCharacteristics));
+      }
----------------
80 cols


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70606/new/

https://reviews.llvm.org/D70606