LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 35739 - compiler-rt: files contain writable and executable sections
Summary: compiler-rt: files contain writable and executable sections
Status: RESOLVED FIXED
Alias: None
Product: compiler-rt
Classification: Unclassified
Component: compiler-rt (show other bugs)
Version: unspecified
Hardware: PC Linux
: P normal
Assignee: Dimitry Andric
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-12-24 10:51 PST by Will Wiemann
Modified: 2017-12-25 03:34 PST (History)
4 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Will Wiemann 2017-12-24 10:51:48 PST
My Linux system detected some writeable and executable sections in the files

usr/lib/clang/5.0.1/lib/linux/libclang_rt.builtins-x86_64.a:chkstk.S.o
usr/lib/clang/5.0.1/lib/linux/libclang_rt.builtins-x86_64.a:chkstk2.S.o
usr/lib/clang/5.0.1/lib/linux/libclang_rt.builtins-i386.a:chkstk.S.o
usr/lib/clang/5.0.1/lib/linux/libclang_rt.builtins-i386.a:chkstk2.S.o

I do not understand entirely why this is the case. However, I'm not alone with this  (see related issues by other projects).

This is what I've found out so far.

The commit [1] fixed part of this but did not cover {i386,x86_64}/chkstk.S, {i386,x86_64}/chkstk2.S. The reason might be that NO_EXEC_STACK_DIRECTIVE is a NO-OP on Windows and according to the comments in the files chkstk.S and chkstk2.S are windows only. This impression is supported by lib/builtin/CMakeFiles.txt which explicitly includes these files as additional x86_64_SOURCES (line 247).
However, these files are already included in x86_64_SOURCES (same for i386_SOURCES). This was done by commit [2]. Looking at this commit more closely, it  adds chkstk.S and chkstk2.S unconditionally even though they were conditionally included. My guess is that these files were added accidentally by [2] and should only be compiled on Windows.

[1]
git-sha: 4c71a477ac0bd7b19c72d7625861f1f18c15b278
git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@273500 91177308-0d34-0410-b5e6-96231b3b80d8

[2]
git-sha: a405fc16c3227898f22422a045ee2e36bc605776
git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@252927 91177308-0d34-0410-b5e6-96231b3b80d8

Related issues by other projects:

rust: https://github.com/rust-lang-nursery/compiler-builtins/issues/183
gentoo: https://bugs.gentoo.org/show_bug.cgi?id=641026
Comment 1 Dimitry Andric 2017-12-24 11:54:18 PST
The chkstk.S and chkstk2.S files are missing a NO_EXEC_STACK_DIRECTIVE line, which causes the files to have an executable stack by default. I have put up a review to fix this: https://reviews.llvm.org/D41567
Comment 2 Dimitry Andric 2017-12-24 12:07:51 PST
In addition, the files might be excluded from non-Windows builds, but the way the files are added in lib/builtins/CMakeLists.txt seems a bit strange to me:

if (NOT MSVC)
  set(x86_64_SOURCES
      x86_64/chkstk.S
      x86_64/chkstk2.S
      x86_64/floatdidf.c
      x86_64/floatdisf.c
      x86_64/floatdixf.c
      x86_64/floatundidf.S
      x86_64/floatundisf.S
      x86_64/floatundixf.S)
  filter_builtin_sources(x86_64_SOURCES EXCLUDE x86_64_SOURCES "${x86_64_SOURCES};${GENERIC_SOURCES}")
  set(x86_64h_SOURCES ${x86_64_SOURCES})

  if (WIN32)
    set(x86_64_SOURCES
        ${x86_64_SOURCES}
        x86_64/chkstk.S
        x86_64/chkstk2.S)
  endif()

E.g., first it adds them unconditionally (as long as it's not MSVC), then it adds them for WIN32.  Therefore, if you build this on e.g. Linux or FreeBSD, it still adds them for compilation.

I see that Chris and Martell touched both those parts, any ideas? :)
Comment 3 Martell Malone 2017-12-24 12:28:34 PST
I have not spent any time testing this but it seems like a regression from rL252927.
chkstk.S and chkstk2.S should only be included on WIN32
removing them from the (NOT MSVC) guards seems like the correct move.
Comment 4 Dimitry Andric 2017-12-24 13:17:32 PST
Fixed in https://reviews.llvm.org/rL321431.
Comment 5 Will Wiemann 2017-12-25 03:34:04 PST
Wow, guys that was quick. Thanks!