[PATCH] D120645: [Object] Skip section offset checking for /<XFGHASHMAP>/

[James Henderson via Phabricator via llvm-commits]

jhenderson requested changes to this revision.
jhenderson added a comment.
This revision now requires changes to proceed.

Basically looks fine, although I'm not a Windows archive expert. @mstorsjo might have some other feedback.



================
Comment at: llvm/lib/Object/Archive.cpp:259-260
       return Name;
+    // System libraries from Windows SDK for Window 11 contain this symbol,
+    // it looks like a CFG guard, we just skip it for now.
+    if (Name.equals("/<XFGHASHMAP>/"))
----------------
Some grammar nits.


================
Comment at: llvm/test/tools/llvm-lib/Inputs/xfghashmap-inserter.py:8
+search = bytes(sys.argv[2], 'utf-8')
+xfghashmap = bytes("/<XFGHASHMAP>/", 'utf-8')
+
----------------
I believe you can just do this?


================
Comment at: llvm/test/tools/llvm-lib/xfghashmap-list.test:1-5
+RUN: rm -rf %t && mkdir -p %t
+RUN: llvm-mc -triple=x86_64-pc-windows-msvc -filetype=obj -o %t/a.obj %S/Inputs/a.s
+RUN: llvm-mc -triple=x86_64-pc-windows-msvc -filetype=obj -o %t/b.obj %S/Inputs/b.s
+
+RUN: rm -f %t/xfghash.lib && cd %t
----------------
You can simplify this slightly.


================
Comment at: llvm/test/tools/llvm-lib/xfghashmap-list.test:7
+RUN: llvm-lib /out:xfghashmap.lib a.obj b.obj
+RUN: %python %S/Inputs/xfghashmap-inserter.py xfghashmap.lib b.obj/
+
----------------
Rather than put the python file in a separate file to the test file, you can include it inline as follows:

1) Add comment markers to the start of every RUN and CHECK line, (e.g. `# RUN: ...`).
2) Change the existing comment to use double-comment markers (not strictly needed, but it helps it stand out (i.e. `## This should ...`).
3) Add the contents of the python script to the end of this test file.
4) Reference the script by using `%s` in your python execution.

Also, I'd put a brief comment somewhere, either at the start of the python script, or immediately before this RUN line, explaining what the script does.


================
Comment at: llvm/test/tools/llvm-lib/xfghashmap-list.test:9
+
+# This should print the /<XFGHASHMAP>/ section as well as an .obj one
+RUN: llvm-lib /list %t/xfghashmap.lib | FileCheck %s
----------------
Nit: missing trailing "." at the end of the comment.


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

https://reviews.llvm.org/D120645