[llvm-commits] [patch] Identifying code sections in MCJIT

[Amara Emerson]

A few comments based on a cursory look through the MachO spec[1].

The read-only executable code seems to be standardized to be located in
segments & sections with name __TEXT or __text. However, there is a line
which says "This naming convention is standard, although not required for
the tools to operate correctly." Whether this means that tools should be
able to handle mixed case or the segments/sections can be named completely
differently is unclear.

A potential issue I've found is that the __mod_init_func and __mod_term_func
sections in the __DATA segment can contain functions such as C++ static
constructors.

>From the looks of it, MachO support could do with some work as some of it
remains stubbed. A new function like ObjectFile::isCode() could be
implemented instead. This would make use of the existing isText() on ELF,
and on MachO check if the section name is __text or one of the special
sections mentioned above.

Amara

[1]
https://developer.apple.com/library/mac/#documentation/developertools/concep
tual/MachORuntime/Reference/reference.html

-----Original Message-----
From: llvm-commits-bounces at cs.uiuc.edu
[mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Kaylor, Andrew
Sent: 11 December 2012 16:53
To: Tim Northover; llvm-commits
Subject: Re: [llvm-commits] [patch] Identifying code sections in MCJIT

Hi Tim,

I'm not sure about this one.  I like your change, but I'm not sure it will
work everywhere right now.

The MachO implementation of 'isText' (actually isSectionText by the time it
gets resolved) checks to see if the section is named "__text" which probably
isn't sufficient for the way you're using it here.  The ELF implementation
checks section flags to see if the section is executable.

I'm not familiar enough with MachO to know how big the risk is here or how
difficult it would be to correctly implement the isText function there.  I
would expect that in most cases your patch would work for MachO.  It might
not work in all cases, but apparently the existing code doesn't work in all
cases either.

I also notice that there are some cases where findOrEmitSection is being
called with the IsCode argument hard-coded to 'true' and one other place
where it is already doing what your patch does.

I'd like to hear from someone who is more familiar with MachO before we
proceed, but I think your patch is a step in the right direction.

-Andy

-----Original Message-----
From: llvm-commits-bounces at cs.uiuc.edu
[mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Tim Northover
Sent: Tuesday, December 11, 2012 5:23 AM
To: llvm-commits
Subject: [llvm-commits] [patch] Identifying code sections in MCJIT

Hi,

After investigating why adding mapping symbols on ARM (r169609) caused
segfaults in the MCJIT, I discovered that the .text section wasn't being
allocated as code in the MemoryManager, and hence wasn't being marked
executable before jumping there.

This was because RuntimeDyldImpl::loadObject was choosing the type based on
whether the first symbol it encountered in that section was ST_Function or
not. Previously it was, but now the mapping symbols (required to have
STT_NOTYPE) are seen first and .text is misclassified.

This patch instead uses the SectionRef method "isText" to make that
decision. It parallels what RuntimeDyldELF does while processing DWARF, and
would seem to be a more sensible API anyway.

Is this correct for MachO too, and is the patch OK to commit?

Cheers.

Tim.

_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits