Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression with -D_FORTIFY_SOURCE=2 #61691

Closed
tstellar opened this issue Mar 24, 2023 · 9 comments
Closed

Regression with -D_FORTIFY_SOURCE=2 #61691

tstellar opened this issue Mar 24, 2023 · 9 comments
Assignees
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" miscompilation regression release:backport

Comments

@tstellar
Copy link
Collaborator

tstellar commented Mar 24, 2023

There was a regression compiling libmbim between clang-14 and clang-15, and it's still present in main.

The bad behavior began after bfb9b8e, however, I think the root cause is a bug in Clang's handling of -D_FORTIFY_SOURCE=2. When building libmbim, the libmbim-glib-scan utility hangs, and from some limited debugging I did, it appears that it gets stuck in a loop of strncpy calling itself, which I think has happened before due to inlining of the *_chk functions in glibc.

I am able to reliably reproduce this bug in a Fedora 37 container using the script below:

sudo dnf install 'dnf-command(builddep)' git
sudo dnf builddep libmbim
git clone https://github.com/freedesktop/libmbim
cd libmbim
git checkout 1.26.4

export CFLAGS='-O2 -flto -g -Wp,-D_FORTIFY_SOURCE=2 -Wno-cast-align -gdwarf-4'
export LDFLAGS='-flto -fuse-ld=lld'
export CC=clang
export AR=llvm-ar
export RANLIB=llvm-ranlib
export NM=llvm-nm

./autogen.sh

./configure --build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu --program-prefix= --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/var/lib --mandir=/usr/share/man --infodir=/usr/share/info --disable-static --enable-gtk-doc

/usr/bin/make -O -j8 V=1 VERBOSE=1

@serge-sans-paille

@efriedma-quic
Copy link
Collaborator

efriedma-quic commented Mar 24, 2023

Clang has to do some weird stuff to "correctly" lower the strange functions that call themselves defined by glibc _FORTIFY_SOURCE. I think this was last touched by https://reviews.llvm.org/D134362 ? We constantly run into edge cases involving this kind of code.

Could use a reduced testcase.

@tstellar
Copy link
Collaborator Author

tstellar commented Mar 25, 2023

Here is a reduced testcase

Compile with

clang -flto -x c -std=gnu89 -O2 -S -emit-llvm reduced.ii -o reduced.ll

The generated strncpy function looks wrong:

; Function Attrs: alwaysinline nofree nounwind willreturn memory(argmem: readwrite) uwtable
define available_externally ptr @strncpy(ptr noalias noundef nonnull returned writeonly %0, ptr noalias nocapture noundef nonnull readonly %1, i64 noundef %2) local_unnamed_addr #3 {
  br label %4

4:                                                ; preds = %4, %3
  %5 = phi ptr [ poison, %3 ], [ %7, %4 ]
  %6 = phi i1 [ false, %3 ], [ true, %4 ]
  %7 = select i1 %6, ptr %5, ptr %0
  br label %4
}

@serge-sans-paille
Copy link
Collaborator

https://reviews.llvm.org/D147307 << potential fix.

@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" and removed new issue labels Mar 31, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 31, 2023

@llvm/issue-subscribers-clang-frontend

tstellar added a commit to fedora-llvm-team/clang-built-fedora-status that referenced this issue Apr 6, 2023
@nikic nikic reopened this Apr 13, 2023
@nikic
Copy link
Contributor

nikic commented Apr 13, 2023

/cherry-pick fd8745c

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 13, 2023

/branch llvm/llvm-project-release-prs/issue61691

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 13, 2023

/pull-request llvm/llvm-project-release-prs#417

flemairen6 pushed a commit to Xilinx/llvm-project that referenced this issue May 10, 2023
@tstellar tstellar removed this from the LLVM 16.0.X Release milestone May 30, 2023
@tstellar
Copy link
Collaborator Author

Since this commit introduced a regression, we will not be backporting it to 16.0.x.

@nikic
Copy link
Contributor

nikic commented Jun 1, 2023

Closing this as the issue has been fixed and this was now tracking the backport -- with the backport declined, we can close this again.

@nikic nikic closed this as completed Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" miscompilation regression release:backport
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants