ObjectWriter update. #2876
ObjectWriter update. #2876
Conversation
sandreenko
force-pushed
the
ObjectWriterUpdate
branch
from
9726826 to
7709013
Compare
|
@MichalStrehovsky, @nattress could you take a look please? |
| SetCodeSectionAttribute(_nativeObjectWriter, section.Name); | ||
| } | ||
|
|
||
| _currentSection = section; |
There was a problem hiding this comment.
Is this actually changing the current section being emitted? If not, we should not alter _currentSection
| managedCodeSection = MethodCodeNode.UnixContentSection; | ||
| objectWriter.SetSection(LsdaSection); | ||
| } | ||
| objectWriter.SetCodeSectionAttribute(managedCodeSection); |
There was a problem hiding this comment.
Is it necessary to add a new ObjectWriter API for this? We have the existing CustomSectionAttributes flags we pass through when setting a section that objectwriter could look for. We could key off CustomSectionAttributes.Executable for instance.
There was a problem hiding this comment.
Yes, it is possible. But I think we should check a contract for CustomSectionAttributes.Executable.
Should Windows sentinels have executable attribute? Do they contain instructions?
Should .text section be IsStandardSection, if it has executable attribute set on and we don't pass it right now.
There was a problem hiding this comment.
The sentinels do not contain instructions. It makes sense to have standard sections also have the correct attributes instead of just for custom sections so things are more consistent. I filed issue #2913 for this.
sandreenko
force-pushed
the
ObjectWriterUpdate
branch
from
418d8f7 to
2e4b948
Compare
| if (factory.Target.OperatingSystem == TargetOS.OSX) | ||
| { | ||
| // TODO fix: we set the section, but do not emit anything, | ||
| // but the generic test fails without this code. |
There was a problem hiding this comment.
It's not clear what this TODO tracks. Is this an issue that should be filed? What are the next steps?
the generic test fails might be a clear thing right now, but 6 months from now, maybe the test won't even exist anymore.
sandreenko
force-pushed
the
ObjectWriterUpdate
branch
from
2e4b948 to
99af593
Compare
| { | ||
| objectWriter.SetSection(MethodCodeNode.UnixContentSection); | ||
| managedCodeSection = MethodCodeNode.UnixContentSection; | ||
| if (factory.Target.OperatingSystem == TargetOS.OSX) |
There was a problem hiding this comment.
Why cannot we do this unconditionally? Is there any downside for other Unixes?
There was a problem hiding this comment.
We can, but we don't have to.
There was a problem hiding this comment.
If there is no downside, then I'd prefer simpler code without the conditional.
sandreenko
force-pushed
the
ObjectWriterUpdate
branch
from
99af593 to
dc53f6c
Compare
Code sections should have special HasInstructions flag set. It is done for .text inside objectWriter.
sandreenko
force-pushed
the
ObjectWriterUpdate
branch
from
dc53f6c to
7efaaa8
Compare
5 participants
Code sections should have special HasInstructions flag set.
It is done for .text inside objectWriter.