GCC compatibility fixes #7 #22810

franksinankaya · 2019-02-23T03:44:08Z

No description provided.

franksinankaya · 2019-02-23T03:44:27Z

src/ilasm/CMakeLists.txt

franksinankaya · 2019-02-25T15:46:42Z

Any review feedback?

janvorli

Besides the nit, I would like to know @noahfalk's preference on the SyntheticStorage stuff change (as I have asked him in the other PR), but it seems fine otherwise.

src/vm/amd64/jithelpers_fastwritebarriers.S

franksinankaya · 2019-02-25T21:21:20Z

@dotnet-bot test coreclr-ci

Suppress warning during hash add casting

src/vm/codeversion.h:112:16: warning: ‘struct NativeCodeVersion::<anonymous union>::SyntheticStorage’ invalid; an anonymous union can only have non-static data members [-fpermissive] struct SyntheticStorage

Remove extra class declaration

src/vm/amd64/virtualcallstubcpu.hpp:735:103: warning: suggest parentheses around ‘-’ in operand of ‘&’ [-Wparentheses] resolveInit.toMiss1 = offsetof(ResolveStub,miss)-(offsetof(ResolveStub,toMiss1)+1) & 0xFF; ^ src/vm/amd64/virtualcallstubcpu.hpp:741:103: warning: suggest parentheses around ‘-’ in operand of ‘&’ [-Wparentheses] resolveInit.toMiss2 = offsetof(ResolveStub,miss)-(offsetof(ResolveStub,toMiss2)+1) & 0xFF; Add parenthesis src/vm/dataimage.cpp:631:55: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses] previousRvaInfo->rva == rvaInfo->rva && previousRvaInfo->size >= rvaInfo->size Add parenthesis src/debug/daccess/daccess.cpp:6871:29: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses] _ASSERTE(peFile == NULL && reflectionModule != NULL || peFile != NULL && reflectionModule == NULL); Add parenthesis src/vm/dataimage.cpp:631:57: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses] (previousRvaInfo->rva == rvaInfo->rva) && (previousRvaInfo->size >= rvaInfo->size)

src/ilasm/method.cpp:35:36: warning: operation on ‘((Method*)this)->Method::m_ulColumns[0]’ may be undefined [-Wsequence-point] m_ulColumns[0]=m_ulColumns[0]=0;

franksinankaya · 2019-02-26T02:49:07Z

Rebased to master.

franksinankaya · 2019-02-26T04:30:38Z

Should this go in since all tests pass now?

Any other review feedback?

franksinankaya · 2019-02-26T04:31:30Z

@noahfalk : see this question:

'I would like to know @noahfalk's preference on the SyntheticStorage stuff change (as I have asked him in the other PR), but it seems fine otherwise.'

noahfalk · 2019-02-26T07:51:55Z

Thanks for pinging, I missed the original in the noise : ) I didn't see if there multiple options you were trying to get my preference between, but what you have here that makes the struct nameless seems fine to me.

cc @kouvel (the current owner of the code version manager)

kouvel · 2019-02-26T10:13:18Z

Looks ok to me as well

This commit fixes coreclr to build in newer versions of llvm (tested with llvm 8 on Fedora 30). These recent versions of llvm (as well as GCC) do not accept values like "20h" as valid integer literals: src/debug/ee/amd64/dbghelpers.S:32:21: error: unknown token in expression add rsp, 20h ^ This was reported as a bug to llvm upstream and they explicitly rejected supporting these literals: https://reviews.llvm.org/D59810 This is partial backport of cbd672e (PR dotnet#22810), which was about adding compatiblity with GCC 5.

This commit fixes coreclr to build in newer versions of llvm (tested with llvm 8 on Fedora 30). These recent versions of llvm (as well as GCC) do not accept values like "20h" as valid integer literals: src/debug/ee/amd64/dbghelpers.S:32:21: error: unknown token in expression add rsp, 20h ^ This was reported as a bug to llvm upstream and they explicitly rejected supporting these literals: https://reviews.llvm.org/D59810 This is partial backport of cbd672e (PR dotnet#22810), with some modifications to compile, which was about adding compatiblity with GCC 5.

This commit fixes coreclr to build in newer versions of llvm (tested with llvm 8 on Fedora 30). These recent versions of llvm (as well as GCC) do not accept values like "20h" as valid integer literals: src/debug/ee/amd64/dbghelpers.S:32:21: error: unknown token in expression add rsp, 20h ^ This was reported as a bug to llvm upstream and they explicitly rejected supporting these literals: https://reviews.llvm.org/D59810 This is partial backport of cbd672e (PR dotnet#22810), with some modifications to compile, which was about adding compatibility with GCC 5.

This commit fixes coreclr to build in newer versions of llvm (tested with llvm 8 on Fedora 30). These recent versions of llvm (as well as GCC) do not accept values like "20h" as valid integer literals: src/debug/ee/amd64/dbghelpers.S:32:21: error: unknown token in expression add rsp, 20h ^ This was reported as a bug to llvm upstream and they explicitly rejected supporting these literals: https://reviews.llvm.org/D59810 This is partial backport of cbd672e (PR #22810), with some modifications to compile, which was about adding compatibility with GCC 5.

* Use thread_local for thread local storage on non MSVC targets * Use local copy of visitor rather than function parameter * Remove extra class qualifier * Replace hex number representation in ASM files * Reorder STDAPI and DLLEXPORT * Suppress conversion Suppress warning during hash add casting * Remove anonymous struct src/vm/codeversion.h:112:16: warning: ‘struct NativeCodeVersion::<anonymous union>::SyntheticStorage’ invalid; an anonymous union can only have non-static data members [-fpermissive] struct SyntheticStorage * Remove class declaration Remove extra class declaration * Remove extern C * Add implicit paranthesis src/vm/amd64/virtualcallstubcpu.hpp:735:103: warning: suggest parentheses around ‘-’ in operand of ‘&’ [-Wparentheses] resolveInit.toMiss1 = offsetof(ResolveStub,miss)-(offsetof(ResolveStub,toMiss1)+1) & 0xFF; ^ src/vm/amd64/virtualcallstubcpu.hpp:741:103: warning: suggest parentheses around ‘-’ in operand of ‘&’ [-Wparentheses] resolveInit.toMiss2 = offsetof(ResolveStub,miss)-(offsetof(ResolveStub,toMiss2)+1) & 0xFF; Add parenthesis src/vm/dataimage.cpp:631:55: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses] previousRvaInfo->rva == rvaInfo->rva && previousRvaInfo->size >= rvaInfo->size Add parenthesis src/debug/daccess/daccess.cpp:6871:29: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses] _ASSERTE(peFile == NULL && reflectionModule != NULL || peFile != NULL && reflectionModule == NULL); Add parenthesis src/vm/dataimage.cpp:631:57: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses] (previousRvaInfo->rva == rvaInfo->rva) && (previousRvaInfo->size >= rvaInfo->size) * Initialize member 1 src/ilasm/method.cpp:35:36: warning: operation on ‘((Method*)this)->Method::m_ulColumns[0]’ may be undefined [-Wsequence-point] m_ulColumns[0]=m_ulColumns[0]=0; * Remove unknown compiler option * Abstract DLLEXPORT Commit migrated from dotnet/coreclr@cbd672e

franksinankaya force-pushed the gnu_cleanup_7 branch 3 times, most recently from e56dcce to a0379c5 Compare February 23, 2019 05:03

am11 reviewed Feb 23, 2019

View reviewed changes

src/ilasm/CMakeLists.txt Outdated Show resolved Hide resolved

franksinankaya force-pushed the gnu_cleanup_7 branch from a0379c5 to 8cc2dee Compare February 23, 2019 19:22

am11 mentioned this pull request Feb 24, 2019

Fix comparison and narrowing errors reported by GCC #22821

Merged

janvorli reviewed Feb 25, 2019

View reviewed changes

src/vm/amd64/jithelpers_fastwritebarriers.S Outdated Show resolved Hide resolved

franksinankaya force-pushed the gnu_cleanup_7 branch 2 times, most recently from 1505e26 to 267db3d Compare February 25, 2019 17:34

franksinankaya added 13 commits February 26, 2019 02:47

Remove extra class qualifier

1136ca7

Replace hex number representation in ASM files

bdd0408

Use local copy of visitor rather than function parameter

4392c24

Reorder STDAPI and DLLEXPORT

dae93b5

Suppress conversion

1c54f67

Suppress warning during hash add casting

Remove anonymous struct

c14d568

src/vm/codeversion.h:112:16: warning: ‘struct NativeCodeVersion::<anonymous union>::SyntheticStorage’ invalid; an anonymous union can only have non-static data members [-fpermissive] struct SyntheticStorage

Remove class declaration

0101067

Remove extra class declaration

Remove extern C

c119ccf

Initialize member 1

8ad922c

src/ilasm/method.cpp:35:36: warning: operation on ‘((Method*)this)->Method::m_ulColumns[0]’ may be undefined [-Wsequence-point] m_ulColumns[0]=m_ulColumns[0]=0;

Remove unknown compiler option

a9038cf

Use thread_local for thread local storage on non MSVC targets

7285867

Abstract DLLEXPORT

9489493

franksinankaya force-pushed the gnu_cleanup_7 branch from 267db3d to 9489493 Compare February 26, 2019 02:48

janvorli merged commit cbd672e into dotnet:master Feb 26, 2019

franksinankaya deleted the gnu_cleanup_7 branch February 26, 2019 16:05

omajid mentioned this pull request Aug 12, 2019

[release/2.1] Replace hex number representation in ASM files #26136

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

GCC compatibility fixes #7 #22810

GCC compatibility fixes #7 #22810

Uh oh!

franksinankaya commented Feb 23, 2019

Uh oh!

franksinankaya commented Feb 23, 2019

Uh oh!

Uh oh!

franksinankaya commented Feb 25, 2019

Uh oh!

janvorli left a comment

Uh oh!

Uh oh!

franksinankaya commented Feb 25, 2019

Uh oh!

franksinankaya commented Feb 26, 2019

Uh oh!

franksinankaya commented Feb 26, 2019

Uh oh!

franksinankaya commented Feb 26, 2019

Uh oh!

noahfalk commented Feb 26, 2019

Uh oh!

kouvel commented Feb 26, 2019

Uh oh!

Uh oh!

GCC compatibility fixes #7 #22810

GCC compatibility fixes #7 #22810

Uh oh!

Conversation

franksinankaya commented Feb 23, 2019

Uh oh!

franksinankaya commented Feb 23, 2019

Uh oh!

Uh oh!

franksinankaya commented Feb 25, 2019

Uh oh!

janvorli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

franksinankaya commented Feb 25, 2019

Uh oh!

franksinankaya commented Feb 26, 2019

Uh oh!

franksinankaya commented Feb 26, 2019

Uh oh!

franksinankaya commented Feb 26, 2019

Uh oh!

noahfalk commented Feb 26, 2019

Uh oh!

kouvel commented Feb 26, 2019

Uh oh!

Uh oh!