Skip to content

Commit

Permalink
[release/6.0] Fix build with Clang 13 (#63314)
Browse files Browse the repository at this point in the history
* Fix clang 13 induced runtime issues (#62170)

The clang 13 optimizer started to assume that "this" pointer is always
properly aligned. That lead to elimination of some code that was actually
needed.
It also takes pointer aliasing rules more strictly in one place in jit.
That caused the optimizer to falsely assume that a callee with an argument
passed by reference is not modifying that argument and used a stale
copy of the original value at the caller site.

This change fixes both of the issues. With this fix, runtime compiled
using clang 13 seems to be fully functional.

* Fix build with clang 13 (#60328)
  • Loading branch information
janvorli authored Jan 4, 2022
1 parent 45d1f9e commit f86caa5
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 12 deletions.
11 changes: 7 additions & 4 deletions eng/native/configurecompiler.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -340,15 +340,17 @@ if (CLR_CMAKE_HOST_UNIX)
#These seem to indicate real issues
add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-Wno-invalid-offsetof>)

if (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
add_compile_options(-Wno-unused-but-set-variable)

if (CMAKE_C_COMPILER_ID MATCHES "Clang")
add_compile_options(-Wno-unknown-warning-option)

# The -ferror-limit is helpful during the porting, it makes sure the compiler doesn't stop
# after hitting just about 20 errors.
add_compile_options(-ferror-limit=4096)

# Disabled warnings
add_compile_options(-Wno-unused-private-field)
# Explicit constructor calls are not supported by clang (this->ClassName::ClassName())
add_compile_options(-Wno-microsoft)
# There are constants of type BOOL used in a condition. But BOOL is defined as int
# and so the compiler thinks that there is a mistake.
add_compile_options(-Wno-constant-logical-operand)
Expand All @@ -363,8 +365,9 @@ if (CLR_CMAKE_HOST_UNIX)
# to a struct or a class that has virtual members or a base class. In that case, clang
# may not generate the same object layout as MSVC.
add_compile_options(-Wno-incompatible-ms-struct)

add_compile_options(-Wno-reserved-identifier)
else()
add_compile_options(-Wno-unused-but-set-variable)
add_compile_options(-Wno-unknown-pragmas)
add_compile_options(-Wno-uninitialized)
add_compile_options(-Wno-strict-aliasing)
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/inc/corhlpr.h
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ struct COR_ILMETHOD_SECT
const COR_ILMETHOD_SECT* Next() const
{
if (!More()) return(0);
return ((COR_ILMETHOD_SECT*)(((BYTE *)this) + DataSize()))->Align();
return ((COR_ILMETHOD_SECT*)Align(((BYTE *)this) + DataSize()));
}

const BYTE* Data() const
Expand Down Expand Up @@ -374,9 +374,9 @@ struct COR_ILMETHOD_SECT
return((AsSmall()->Kind & CorILMethod_Sect_FatFormat) != 0);
}

const COR_ILMETHOD_SECT* Align() const
static const void* Align(const void* p)
{
return((COR_ILMETHOD_SECT*) ((((UINT_PTR) this) + 3) & ~3));
return((void*) ((((UINT_PTR) p) + 3) & ~3));
}

protected:
Expand Down Expand Up @@ -579,7 +579,7 @@ typedef struct tagCOR_ILMETHOD_FAT : IMAGE_COR_ILMETHOD_FAT

const COR_ILMETHOD_SECT* GetSect() const {
if (!More()) return (0);
return(((COR_ILMETHOD_SECT*) (GetCode() + GetCodeSize()))->Align());
return(((COR_ILMETHOD_SECT*) COR_ILMETHOD_SECT::Align(GetCode() + GetCodeSize())));
}
} COR_ILMETHOD_FAT;

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/bitsetasshortlong.h
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ class BitSetOps</*BitSetType*/ BitSetShortLongRep,
{
if (IsShort(env))
{
(size_t&)out = (size_t)out & ((size_t)gen | (size_t)in);
out = (BitSetShortLongRep)((size_t)out & ((size_t)gen | (size_t)in));
}
else
{
Expand All @@ -361,7 +361,7 @@ class BitSetOps</*BitSetType*/ BitSetShortLongRep,
{
if (IsShort(env))
{
(size_t&)in = (size_t)use | ((size_t)out & ~(size_t)def);
in = (BitSetShortLongRep)((size_t)use | ((size_t)out & ~(size_t)def));
}
else
{
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/inlinepolicy.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ class DefaultPolicy : public LegalPolicy
class ExtendedDefaultPolicy : public DefaultPolicy
{
public:
ExtendedDefaultPolicy::ExtendedDefaultPolicy(Compiler* compiler, bool isPrejitRoot)
ExtendedDefaultPolicy(Compiler* compiler, bool isPrejitRoot)
: DefaultPolicy(compiler, isPrejitRoot)
, m_ProfileFrequency(0.0)
, m_BinaryExprWithCns(0)
Expand Down
3 changes: 3 additions & 0 deletions src/libraries/Native/Unix/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ add_compile_options(-Wno-cast-align)
add_compile_options(-Wno-typedef-redefinition)
add_compile_options(-Wno-c11-extensions)
add_compile_options(-Wno-unknown-pragmas)
add_compile_options(-Wno-unknown-warning-option)
add_compile_options(-Wno-unused-but-set-variable)

check_c_compiler_flag(-Wimplicit-fallthrough COMPILER_SUPPORTS_W_IMPLICIT_FALLTHROUGH)
if (COMPILER_SUPPORTS_W_IMPLICIT_FALLTHROUGH)
Expand All @@ -48,6 +50,7 @@ add_compile_options(-g)
if(CMAKE_C_COMPILER_ID STREQUAL Clang)
add_compile_options(-Wthread-safety)
add_compile_options(-Wno-thread-safety-analysis)
add_compile_options(-Wno-reserved-identifier)
elseif(CMAKE_C_COMPILER_ID STREQUAL GNU)
add_compile_options(-Wno-stringop-truncation)
endif()
Expand Down
20 changes: 19 additions & 1 deletion src/libraries/Native/Unix/System.Native/pal_process.c
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,24 @@ static int SetGroups(uint32_t* userGroups, int32_t userGroupsLength, uint32_t* p
return rv;
}

typedef void (*VoidIntFn)(int);

static
VoidIntFn
handler_from_sigaction (struct sigaction *sa)
{
if (((unsigned int)sa->sa_flags) & SA_SIGINFO)
{
// work around -Wcast-function-type
void (*tmp)(void) = (void (*)(void))sa->sa_sigaction;
return (void (*)(int))tmp;
}
else
{
return sa->sa_handler;
}
}

int32_t SystemNative_ForkAndExecProcess(const char* filename,
char* const argv[],
char* const envp[],
Expand Down Expand Up @@ -371,7 +389,7 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename,
}
if (!sigaction(sig, NULL, &sa_old))
{
void (*oldhandler)(int) = (((unsigned int)sa_old.sa_flags) & SA_SIGINFO) ? (void (*)(int))sa_old.sa_sigaction : sa_old.sa_handler;
void (*oldhandler)(int) = handler_from_sigaction (&sa_old);
if (oldhandler != SIG_IGN && oldhandler != SIG_DFL)
{
// It has a custom handler, put the default handler back.
Expand Down

0 comments on commit f86caa5

Please sign in to comment.