[PATCH] D48690: lld-link: align sections to 16 bytes if referenced from the gfids table

[Hans Wennborg via Phabricator via llvm-commits]

hans accepted this revision.
hans added inline comments.


================
Comment at: lld/test/COFF/guardcf-align.s:11
+# CHECK: 0x{{[0-9A-Fa-f]+0$}}
+# CHECK-NOT: 0x{{[0-9A-Fa-f]+[^0]$}}
+# CHECK: ]
----------------
inglorion wrote:
> hans wrote:
> > It would be good to have a comment about the intention of these checks, since it's not obvious from a quick read.
> > 
> > The regexes are a little tricky to read. Will there be more entries in the gfid than the one for foo? If not, could we just check that there's only one entry and that it's aligned?
> > 
> > Also it would be nice to check that it's really foo that's getting aligned. Maybe we could generate a map file  (/lldmap:file) and check foo's alignment there?
> > 
> > Oh, and could we be getting lucky and foo is aligned because it's first in the segment or something? Maybe there should be more than one function that we check the alignment of just to be sure?
> I've added a comment to explain what we're looking for with the regexes.
> 
> The table actually contains multiple entries. We're checking that all of them end in 0 (which means they're 16-byte aligned). We could constrain the check to only foo (using the map, as you suggest), but really we want all of them to be aligned. foo is just a symbol in a section specifically crafted to cause alignment problems.
> 
> You're right that foo could end up being aligned by accident, but it's unlikely that all entries will be aligned by accident. In fact, they aren't, without the patch.
Okay, sounds good.


https://reviews.llvm.org/D48690