LLVM does not generate correct code on arm

[llvmbot]

Bugzilla Link	34583
Resolution	INVALID
Resolved on	Sep 28, 2017 15:05
Version	trunk
OS	Linux
Blocks	#24719
Attachments	The pre-processed file., asm from the bad compiler, good_asm built by LLVM with that patch reverted.
Reporter	LLVM Bugzilla Contributor
CC	@stephenhines

Extended Description

We found a LLVM change makes one of our test fail.
https://bugs.chromium.org/p/chromium/issues/detail?id=758878&q=owner%3Ame&colspec=ID%20Pri%20M%20Stars%20ReleaseBlock%20Component%20Status%20Owner%20Summary%20OS%20Modified

The mis-compiled function is show below. If I remove the assertions at the beginning of this function, the test passes.

I put the pre-processed code in the attachment.
The command line to compile the code is
/usr/bin/clang --sysroot=/build/kevin -Qunused-arguments -grecord-gcc-switches -fstack-protector-strong -pie -fno-omit-frame-pointer -D_FORTIFY_SOURCE=2 -mthumb -c -MD -MF bsdrm/src/egl.pic.d -O2 -O2 -pipe -march=armv8-a+crc -mtune=cortex-a57.cortex-a53 -mfpu=crypto-neon-fp-armv8 -mfloat-abi=hard -g -fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables -I/build/kevin/usr/include/libdrm -DEGL_EGLEXT_PROTOTYPES -DGL_GLEXT_PROTOTYPES -Wall -Werror -fno-strict-aliasing -Wformat=2 -fstack-protector-strong -fvisibility=internal -ggdb3 -Wa,--noexecstack -O2 -std=gnu99 -I/build/kevin/tmp/portage/chromeos-base/drm-tests-0.0.1-r100/work/drm-tests-0.0.1/bsdrm/include -I/build/kevin/usr/include/libdrm -DEGL_EGLEXT_PROTOTYPES -DGL_GLEXT_PROTOTYPES -DUSE_ATOMIC_API -D_FORTIFY_SOURCE=2 -fPIC -o a.e test.i -B/usr/libexec/gcc/armv7a-cros-linux-gnueabi -target armv7a-cros-linux-gnueabihf -mfloat-abi=hard

EGLImageKHR bs_egl_image_create_gbm(struct bs_egl *self, struct gbm_bo *bo)
{
assert(self);
assert(self->CreateImageKHR);

    int fds[GBM_MAX_PLANES];

    EGLint khr_image_attrs[37] ={12375, 2400, 12374, 1600, 12913, 875713112, 0};

    initStruct(self, bo, fds,  khr_image_attrs);

    size_t attrs_index = 6;
    for (size_t plane = 0; plane < gbm_bo_get_num_planes(bo) ;  plane++) {
            khr_image_attrs[attrs_index++] = EGL_DMA_BUF_PLANE0_FD_EXT + plane * 3;
            khr_image_attrs[attrs_index++] = fds[plane];
            khr_image_attrs[attrs_index++] = EGL_DMA_BUF_PLANE0_OFFSET_EXT + plane * 3;
            khr_image_attrs[attrs_index++] = gbm_bo_get_plane_offset(bo, plane);
            khr_image_attrs[attrs_index++] = EGL_DMA_BUF_PLANE0_PITCH_EXT + plane * 3;
            khr_image_attrs[attrs_index++] = gbm_bo_get_plane_stride(bo, plane);
            if (self->use_dma_buf_import_modifiers) {
                    const uint64_t modifier = gbm_bo_get_format_modifier(bo);
                    khr_image_attrs[attrs_index++] =
                        EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT + plane * 2;
                    khr_image_attrs[attrs_index++] = modifier & 0xfffffffful;
                    khr_image_attrs[attrs_index++] =
                        EGL_DMA_BUF_PLANE0_MODIFIER_HI_EXT + plane * 2;
                    khr_image_attrs[attrs_index++] = modifier >> 32;
            }
    }

    khr_image_attrs[attrs_index++] = EGL_NONE;

    EGLImageKHR image =
        self->CreateImageKHR(self->display, EGL_NO_CONTEXT, EGL_LINUX_DMA_BUF_EXT,
                             NULL /* no client buffer */, khr_image_attrs);

    return image;

}

[llvmbot]

The patch that introduces this problem is

commit f41c3c9 (HEAD)
Author: Kristof Beyls kristof.beyls@arm.com
Date: Wed Jun 28 07:07:03 2017 +0000

[ARM] Make -mcpu=generic schedule for an in-order core (Cortex-A8).

[kbeyls]

Commit f41c3c9 merely enables instruction scheduling to happen when no -mcpu is specified. Therefore, the most plausible explanation seems to be that this uncovers a latent bug somewhere else in the compiler.

I compiled the preprocessed file with the command line options listed above, both for revision f41c3c9 (r306514 in SVN-speak) and f41c3c9^ (r306513 in SVN-speak).

Apart from the expected slight difference in instruction scheduling, register allocation and spill/fill code, there didn't seem to be any differences.

One wild guess is the slight difference I noticed on the following instruction sequences:
r306514:
....
ldr r1, .LCPI6_2
ldr r2, [sp, #​176]
.LPC6_1:
add r1, pc
ldr r1, [r1]
ldr r1, [r1]
subs r1, r1, r2
bne .LBB6_14
....

r306513:
....
ldr r1, .LCPI6_2
.LPC6_1:
add r1, pc
ldr r1, [r1]
ldr r1, [r1]
ldr r2, [sp, #​184]
subs r1, r1, r2
bne .LBB6_14
....

With both labels LCPI6_1 and LCPI6_2 involved somehow in the stack guard checking:
....
.LCPI6_2:
.Ltmp3:
.long __stack_chk_guard(GOT_PREL)-((.LPC6_1+4)-.Ltmp3)
....

Could it be that the __stack_chk_guard mechanism isn't robust in the face of instruction scheduling?
This is just a wild guess, as I haven't seen other differences that seem suspect; and I also have no idea how the __stack_chk_guard mechanism actually works.
Does the "ldr rX, .LCPI6_Y" need to be followed immediately by the .LPC6_Z label?
What makes me think this might be a plausible potential cause is that you use -fstack-protector-strong which is a command line option which maybe others don't use that much, explaining why this issue hasn't been reported before.

[kbeyls]

Hmmm looking a bit closer at the code difference I marked in my previous comment, it doesn't look problematic to me after all.

I notice the function that is claimed to be miscompiled also calls quite a few other functions. It is possible the miscompile is actually in any of those other functions?

[llvmbot]

It seems the bug is not related to the CL I mentioned before.

The vanilla (good) compiler also makes the test fail if I split the function.

In addition, if I add a printf-like function call in the splitted function, the test passes.

[llvmbot]

Turns out to be a source code problem.