Page MenuHomePhabricator

[LLD] Implement /guard:[no]ehcont
ClosedPublic

Authored by pengfei on Mon, Mar 22, 7:46 AM.

Diff Detail

Event Timeline

pengfei requested review of this revision.Mon, Mar 22, 7:46 AM
pengfei created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMon, Mar 22, 7:46 AM
pengfei updated this revision to Diff 332576.Tue, Mar 23, 2:19 AM

Add EHCont to load config table.

pengfei added a subscriber: rnk.Tue, Mar 23, 2:30 AM

Hi @rnk , I'm implementing EHCont guard in LLD referring to your longjmp patch. Currently the binary's EHCont table always be null. I guess it results from I haven't told lld where's the place it should put the EHCont table in the load config table.
But I didn't find any related code in longjmp patch. Could you point me where the code is? Or I misunderstood something?

rnk added a comment.Tue, Mar 23, 1:56 PM

Hi @rnk , I'm implementing EHCont guard in LLD referring to your longjmp patch. Currently the binary's EHCont table always be null. I guess it results from I haven't told lld where's the place it should put the EHCont table in the load config table.
But I didn't find any related code in longjmp patch. Could you point me where the code is? Or I misunderstood something?

The CRT is actually responsible for providing a struct defining the load configuration. You can see in the LLD gfids tests that they all have a blob like this:

        .section .rdata,"dr"
.globl _load_config_used
_load_config_used:
        .long 256
        .fill 124, 1, 0
        .quad __guard_fids_table
        .quad __guard_fids_count
        .long __guard_flags
        .fill 128, 1, 0

To add ehcont tests, you will need to provide a similar load config that llvm-readobj can parse. Hopefully that answers your question.

pengfei updated this revision to Diff 333000.Wed, Mar 24, 8:14 AM

Solved the problems.

pengfei retitled this revision from [LLD][WIP] Implement /guard:[no]ehcont to [LLD] Implement /guard:[no]ehcont.Wed, Mar 24, 8:19 AM
pengfei added reviewers: rnk, ruiu.
pengfei added subscribers: LuoYuanke, annita.zhang.
In D99078#2645963, @rnk wrote:

Hi @rnk , I'm implementing EHCont guard in LLD referring to your longjmp patch. Currently the binary's EHCont table always be null. I guess it results from I haven't told lld where's the place it should put the EHCont table in the load config table.
But I didn't find any related code in longjmp patch. Could you point me where the code is? Or I misunderstood something?

The CRT is actually responsible for providing a struct defining the load configuration. You can see in the LLD gfids tests that they all have a blob like this:

        .section .rdata,"dr"
.globl _load_config_used
_load_config_used:
        .long 256
        .fill 124, 1, 0
        .quad __guard_fids_table
        .quad __guard_fids_count
        .long __guard_flags
        .fill 128, 1, 0

To add ehcont tests, you will need to provide a similar load config that llvm-readobj can parse. Hopefully that answers your question.

Yes. It solves my problems. Thanks very much.
Now, the last question: I found it works with a given load configuration in the test (That's why I'm sure it's the right direction), but still doesn't work when using clang-cl or/and lld-link to build a cpp file. I guess it maybe related to the lld-link that doesn't use the latest CRT. Do you know how to fix this problem?

lld/test/COFF/pdb-natvis.test
17 ↗(On Diff #333000)

I have no idea how this been affected by this patch.

pengfei updated this revision to Diff 333197.Wed, Mar 24, 8:19 PM
  1. Remove lib dependency to make linux testing happy.
  2. Revert the change to pdb-natvis.test.
pengfei added inline comments.Wed, Mar 24, 8:25 PM
lld/test/COFF/pdb-natvis.test
17 ↗(On Diff #333000)

Since the change results in the build server fail, I suspect if it is my local problem. Reverted it to see if the build server happy.

rnk accepted this revision.Tue, Apr 13, 3:59 PM

Looks good!

This revision is now accepted and ready to land.Tue, Apr 13, 3:59 PM
In D99078#2687146, @rnk wrote:

Looks good!

Thanks for the review.

This revision was landed with ongoing or failed builds.Wed, Apr 14, 12:07 AM
This revision was automatically updated to reflect the committed changes.