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

[x86/Linux] Fix compile error in codeman#8296

Closed
seanshpark wants to merge 1 commit into
dotnet:masterfrom
seanshpark:x86vm02
Closed

[x86/Linux] Fix compile error in codeman#8296
seanshpark wants to merge 1 commit into
dotnet:masterfrom
seanshpark:x86vm02

Conversation

@seanshpark
Copy link
Copy Markdown

@seanshpark seanshpark commented Nov 24, 2016

Fix compile error for x86/Linux

  • add !TARGET_X86 to WIN64EXCEPTIONS directives
  • fix "use of undeclared identifier 'NEED_TO_PORT_THIS_ONE'"
  • fix "no member named 'GetUnwindInfo' in '_hpCodeHdr'"
  • fix "use of undeclared identifier 'RUNTIME_FUNCTION__EndAddress'"

@RussKeldorph
Copy link
Copy Markdown

Please ignore the x86 compatjit and x86 legacy_backend jobs if they fail. They have been removed from future PRs.

Comment thread src/vm/i386/cgencpu.h Outdated
#define HAS_NDIRECT_IMPORT_PRECODE 1

#ifdef FEATURE_PAL
#define USE_INDIRECT_CODEHEADER
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be left undefined (see my comment about changing the ABI in the other PR).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thank you for point this.

Comment thread src/vm/codeman.cpp Outdated
#if !defined(FEATURE_PAL)
LPCWSTR pwzJitName = MAKEDLLNAME_W(L"compatjit");
#else
LPCWSTR pwzJitName = MAKEDLLNAME_W(u"compatjit");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is W(...) macro to do this without ifdefs.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, found it.

@seanshpark
Copy link
Copy Markdown
Author

If I tried to block LazyGetFunctionEntry function(s) , it lead to no where, so just block the function body not to use GetUnwindInfo() and fix it properly after build is ready.

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Nov 29, 2016

Pretty much all defined(WIN64EXCEPTIONS) in this file should be defined(WIN64EXCEPTIONS) && !defined(_TARGET_X86_) - similar to what you have done under src/zap.

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Nov 29, 2016

Maybe we should invent a new #define for this.

@seanshpark
Copy link
Copy Markdown
Author

invent a new #define

Good to hear :) . Codes seems not about Exception. Could you give a good new directive?

Pretty much all defined(WIN64EXCEPTIONS) in this file should be defined(WIN64EXCEPTIONS) && !defined(_TARGET_X86_)

The change spread to several files, threads.cpp, jitinterface.cpp, eedbginterfaceimpl.cpp and h, stackwalk.cpp and h and maybe so on. It is scary to change not knowing much.

@parjong has also tried fixing this compile error another way so if adding _ASSERTE(!"Fix This"); is inappropriate, I'll change this patch only leaving with Wmacro change.

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Nov 29, 2016

It is scary to change not knowing much.

You may be better off to just make it compile on Linux x86 same way as it compile on Windows x86 (ie without WIN64EXCEPTIONS defined). It should allow you to get hello world running, with much fewer changes. And then try to turn on WIN64EXCEPTIONS as isolated change that is not mixed up with the rest.

@seanshpark
Copy link
Copy Markdown
Author

@ OK Thanks, I'll try again :)

Fix compile error for x86/Linux
- add !_TARGET_X86_ to WIN64EXCEPTIONS directives
- fix "use of undeclared identifier 'NEED_TO_PORT_THIS_ONE'"
- fix "no member named 'GetUnwindInfo' in '_hpCodeHdr'"
- fix "use of undeclared identifier 'RUNTIME_FUNCTION__EndAddress'"
@seanshpark
Copy link
Copy Markdown
Author

Please wait, need to resolve with another PR by parjong.

@janvorli
Copy link
Copy Markdown
Member

You may be better off to just make it compile on Linux x86 same way as it compile on Windows x86 (ie without WIN64EXCEPTIONS defined).

Yeah, I am starting to think the same. I have not realized before that the WIN64EXCEPTIONS is used at that many places in the source.

@seanshpark
Copy link
Copy Markdown
Author

without WIN64EXCEPTIONS defined

Yes, It's getting huge. I'll close this PR and restart from without WIN64EXCEPTIONS

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.

5 participants