Bug 207175 - projects/clang380-import for TARGET_ARCH=powerpc : clang 3.8.0 sometimes mishandles va_list overflow_arg_area vs. reg_save_area
Summary: projects/clang380-import for TARGET_ARCH=powerpc : clang 3.8.0 sometimes mish...
Status: Closed Overcome By Events
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: powerpc Any
: --- Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-14 02:42 UTC by Mark Millard
Modified: 2017-10-11 22:21 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Millard 2016-02-14 02:42:41 UTC
in a projects/clang380-import context libxo code use by "ls -l -n /" gets a segmentation violation (SEGV) for TARGET_ARCH=powerpc for buildworld/installworld. The later simplified code reproduces the problem.

The issue is complicated alignment/placement handling of va_list content between overflow_arg_area and reg_save_area that can happen when things larger than 4 bytes in size are involved in the mix, such as intmax_t.

It can be that only 4 bytes are left in reg_save_area when the next things is, say, 8 bytes in size. The next thing after that can be 4 bytes in size --which place does the 4 byte item go and which is it extracted from?

The code generated is not consistent with itself. The following short program demonstrates the problem.

#include <stdarg.h> // for va_list, va_start, va_arg, va_end
#include <stdint.h> // for intmax_t

intmax_t
va_test (char *s, ...)
{
    va_list vap;

    va_start(vap, s);

    char*        t0 = va_arg(vap, char*);
    unsigned int o0 = va_arg(vap, unsigned int);
    int          c0 = va_arg(vap, int);
    unsigned int u0 = va_arg(vap, unsigned int);
    int          c1 = va_arg(vap, int);
    char *       t1 = va_arg(vap, char*);
 
    intmax_t     j0 = va_arg(vap, intmax_t); // This spans into overflow_arg_area.

    int          c2 = va_arg(vap, int);      // A copy was put in the 
                                             // overflow_arg_area because of the
                                             // above.
                                             // But this tries to extract from the
                                             // last 4 bytes of the reg_save_area.
                                             // It does not increment the
                                             // overflow_arg_area position pointer
                                             // past the copy that is there.

    char *       t2 = va_arg(vap, char*);    // The lack of increment before makes
                                             // this extraction off by 4 bytes.

    char         t2fc = *t2;  // <<< This gets SEGV. t2 actually got what should be
                              //     the c2 value.

    intmax_t     j1 = va_arg(vap, intmax_t);

    va_end(vap);

    return (intmax_t) ((s-t2)+(t0-t1)+o0+u0+j0+j1+c0+c1+c2+t2fc);
    // Avoid any optimize-away for lack of use.
}

int main(void)
{
    char         s[1025] = "test string for this";

    char*        t0 = s + 5;
    unsigned int o0 = 3;
    int          c0 = 1;
    unsigned int u0 = 1;
    int          c1 = 3;
    char *       t1 = s + 12;
    intmax_t     j0 = 314159265358979323;
    int          c2 = 4;
    char *       t2 = s + 16;
    intmax_t     j1 = ~314159265358979323;

    intmax_t      result = va_test(s,t0,o0,c0,u0,c1,t1,j0,c1,t2,j1);

    return (int) (result - (intmax_t) ((s-t2)+(t0-t1)+o0+u0+j0+j1+c0+c1+c2+*t2));
    // Avoid any optimize-away for lack of use.
}

Context:

# freebsd-version -ku; uname -aKU
11.0-CURRENT
11.0-CURRENT
FreeBSD FBSDG4C1 11.0-CURRENT FreeBSD 11.0-CURRENT #3 r295351M: Sat Feb  6 16:27:58 PST 2016     markmi@FreeBSDx64:/usr/obj/clang_gcc421/powerpc.powerpc/usr/src/sys/GENERICvtsc-NODEBUG  powerpc 1100097 1100097
Comment 1 Mark Millard 2016-02-14 02:52:15 UTC
(In reply to Mark Millard from comment #0)

I have submitted this to llvm as 26605 ( https://llvm.org/bugs/show_bug.cgi?id=26605 ).
Comment 2 Mark Millard 2016-02-19 03:21:51 UTC
(In reply to Mark Millard from comment #1)

The following has been submitted to llvm 26605. It matches up with the exchange on the lists.


Roman Divacky (rdivacky at vlakno.cz) came up with the following patch [other than some discussion of what turned into "(8)" in "getIn8(8)" below]. The svnlite diff below is from my testing area for projects/clang380-import in FreeBSD's svn. So the revision number is from there too.

Index: /usr/src/contrib/llvm/tools/clang/lib/CodeGen/TargetInfo.cpp
===================================================================
--- /usr/src/contrib/llvm/tools/clang/lib/CodeGen/TargetInfo.cpp	(revision 295601)
+++ /usr/src/contrib/llvm/tools/clang/lib/CodeGen/TargetInfo.cpp	(working copy)
@@ -3569,6 +3569,8 @@
  {
    CGF.EmitBlock(UsingOverflow);

+    Builder.CreateStore(Builder.getInt8(8), NumRegsAddr);
+
    // Everything in the overflow area is rounded up to a size of at least 4.
    CharUnits OverflowAreaAlign = CharUnits::fromQuantity(4);

The "(8)" is intended to match the "(8)" from:

  llvm::Value *CC =
      Builder.CreateICmpULT(NumRegs, Builder.getInt8(8), "cond");


My simple test cases and FreeBSD's clang380-import "buildworld" materials (built with the udpate clang 3.8.0) all indicate that this fixes the problem reported.

For example each of the following now work based on code generation from the patched clang 3.8.0:

la -l -n /

svnlite update -295601 /usr/src

Both are from using the patched clang 3.8.0 to "buildworld" and then install it and reboot and use the software. The FreeBSD vintage started from is projects/clang380-import -r295601 .

I did some testing with doubles in use as a variant of my simplified example as well in order to test the fpr related handling, not just gpr related. The mix worked fine.


[Note: I run with the signal delivery modified to have a "red zone" to deal with other aspects of clang 3.8.0 code generation that are currently not ABI compliant for when the stack pointer is moved. Having a "red zone" is still operationally correct for an ABI compliant code generation, it just temporarily wastes more bytes. Also: the kernel was built with gcc 4.2.1 but world was built with the patched clang 3.8.0. At this stage I've only been identifying issues with "world" vs. clang 3.8.0. The kernel is a much more involved issue.]
Comment 3 Mark Millard 2016-06-17 04:05:04 UTC
https://llvm.org/bugs/show_bug.cgi?id=26605 got its fix in llvm's 261422 check in.

But it is going to be some time [weeks or months] before I again have access to the powerpcs that I had been using. I'm not currently set up to check 11.0 -r310975 on powerpc's [or powerpc64's].

I expect that this specific issue is fixed.

[But I'll note that there are many other problems for powerpc [and powerpc64] code generation via clang. For powerpc it does not follow the stack handling part of the expected ABI correctly. Testing the fix involves sidestepping the ABI requirement so that clang's generated code is tolerated. With such work arounds I've only gotten as far as buildworld being clang based, not buildkernel.]
Comment 4 Mark Millard 2016-11-18 04:06:39 UTC
(In reply to Mark Millard from comment #3)

I have tested compiling and running va_test.c under the system compiler for:

# uname -apKU
FreeBSD FBSDG4S 12.0-CURRENT FreeBSD 12.0-CURRENT #0 r308247M: Thu Nov  3 11:39:11 PDT 2016     markmi@FreeBSDx64:/usr/obj/powerpcvtsc_clang_gcc421_kernel/powerpc.powerpc/usr/src/sys/GENERICvtsc-NODBG  powerpc powerpc 1200014 1200014

which is still clang 3.8.0 based (like stable/11 and release/11.0).

It does not get the reported SEGV and completes normally.

But this is from my environment for working around other clang problems when it targets powerpc.
Comment 5 Justin Hibbits freebsd_committer freebsd_triage 2017-10-11 21:49:42 UTC
This should've been fixed by newer clang imports.  If not, please reopen with newer data.
Comment 6 Mark Millard 2017-10-11 22:21:28 UTC
(In reply to Justin Hibbits from comment #5)

So far as I know FreeBSD's ABI is being followed
now and is working for var args. I've not observed
any problems in a long time.


Side note:

llvm bugzilla 33108 is about darwin's ABI not being
handled correctly for some aspects of var args.