Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Generate the dac table RVA using nm at build time. #1195

Merged
merged 2 commits into from
Jul 2, 2015

Conversation

mikem8361
Copy link
Member

Remove the temporary PAL dac table file at runtime. Replaced it with dactablerva.h file generated at build time from the libcoreclr module. nm works across Linux, OSx and FreeBSD. Issue #501.

Remove the temporary PAL dac table file at runtime. Replaced it with dactablerva.h file generated at build time from the libcoreclr module. nm works across Linux, OSx and FreeBSD.
@mikem8361
Copy link
Member Author

@Djuffin and @pgavlin can you review?

@Djuffin
Copy link

Djuffin commented Jul 2, 2015

LGTM

@pgavlin
Copy link

pgavlin commented Jul 2, 2015

LGTM as well.

@OtherCrashOverride
Copy link

Doesn't this introduce a version dependency on DAC: If DAC changes, you have to rebuild (make) and recompile everything? Shouldn't this use OS symbol resolution services like dlsym instead?

@mikem8361
Copy link
Member Author

Yes, the DAC and DBI are already very version dependent on the coreclr. Most of the DAC code is the coreclr classes/code rebuilt with some different macros/defines so there is already the tight binding.

We could have still done the dactable copying at runtime via dlsym except then the coreclr module would have to be loaded in the debugger side process just to get the symbol. Since the module can be unloaded that may not be a big deal.

@OtherCrashOverride
Copy link

Also, how well does this method interact with Address Space Layout Randomization?

@Djuffin
Copy link

Djuffin commented Jul 2, 2015

Coreclr module base address is detected at runtime, it's only DAC table offset inside the module that is "hardcoded", that's why randomization shouldn't affect it in any way.

@kangaroo
Copy link

kangaroo commented Jul 2, 2015

That presumes that any ASLR mechanism does not do segment reordering. This is called an RVA, but its not really an RVA. Its a compile time constant offset from 0x0 for the compilation unit. The runtime linker could move things.

@Djuffin
Copy link

Djuffin commented Jul 2, 2015

@kangaroo - good point. In case of segment reordering, loading coreclr into debugger process and calling dlsym() wouldn't help either.

Are there any systems out there known to do segment randomization for shared libraries? We'd like to test it properly.

@kangaroo
Copy link

kangaroo commented Jul 2, 2015

@Djuffin Why would dlsym() not work? dlsym (should) get fixup information from the rt linker. I think you're making assumptions about the dlsym implementation.

I'm not sure if there are public implementations, but I am aware of systems that do segment randomization.

@Djuffin
Copy link

Djuffin commented Jul 2, 2015

Correct me if I'm wrong, but if a managed debugger loads libcoreclr and calls dlsym("g_dacGlobals") on it, this address would in no way help to locate address of g_dacGlobals in a debugee process, assuming segment randomization is working.

As I said before we'd like to avoid requirement of debugee cooperation, when it needs to call dlsym("g_dacGlobals") and publish result so it can be read by a debugger. That's more or less what we have right now, and we're not happy with it.

@kangaroo
Copy link

kangaroo commented Jul 2, 2015

I understand now. I don't think its possible to resolve this without debugee cooperation.

@mikem8361
Copy link
Member Author

@kangaroo, @Djuffin, @OtherCrashOverride

I’m assuming you mean resolve any problems with ASLR. I’m going to go ahead and merge this change because it fixes a bunch of other problems for crashdumps, etc.

mikem8361 added a commit that referenced this pull request Jul 2, 2015
Generate the dac table RVA using nm at build time.
@mikem8361 mikem8361 merged commit 8cfd0cd into dotnet:master Jul 2, 2015
@mikem8361 mikem8361 deleted the dactable branch July 2, 2015 18:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants