Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Richard Smith via cfe-commits]

On 15 August 2017 at 08:56, Diana Picus via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Ugh, I'm not sure that last email made it through.
>
> Here's the IR with -Xclang -disable-llvm-passes, at r310400 and r310401:
> https://goo.gl/4n84SR
> https://goo.gl/gxhxp9
>
> The diff between them is small enough so I don't think there's much
> point in reducing it further.
> Hope that helps.


Yes, that helps a lot. The problem was that the implicitly-generated
va_list struct for ARM wasn't getting the "CanPassInRegisters" flag set,
and got passed indirectly (contrary to the ABI).

I've added a test for that kind of problem in r310982, and re-committed the
patch as r310983.

Thanks for all your assistance here!


> On 15 August 2017 at 17:51, Diana Picus  wrote:
> > Actually, I'm not sure I need to reduce it, the diff between the IR
> > with -Xclang -no-llvm-passes is pretty small for the version
> > with/without the patch. See attached. I hope this helps, it does look
> > like a problem with va_arg handling.
> >
> > Sorry again that this is going so slowly...
> >
> > On 15 August 2017 at 16:52, Diana Picus  wrote:
> >> On 15 August 2017 at 01:25, Richard Smith 
> wrote:
> >>> On 14 August 2017 at 03:27, Diana Picus via cfe-commits
> >>>  wrote:
> 
>  Hi,
> 
>  Strangely enough, it turns out that if I run
>  Asan-armhf-with-calls-Noinst-Test on the command line it fails,
>  although it doesn't fail when run with lit.
> >>>
> >>>
> >>> Looks like the crash is within the "use_colors == true" portion of
> >>> ColoredPrintf, so this would make sense if running the test within lit
> turns
> >>> off color support (perhaps because the output is not a terminal).
> >>>
> >>
> >> That's a good catch, it seems if I run it with --gtest-color=no it
> >> still passes, but prints some garbage at the end:
> >> [ PASSED ] 97 tests.
> >> YOU HAVE -1094542056 DISABLED F]�뀼��_8�pGD2
> >>
> >>
> 
>  I've attached the stack
>  trace from gdb. It looks like some trouble passing down va_arg
>  parameters, but I haven't looked into too much details. The segfault
>  happens when we try to do a   ldrb   r3, [r0, r1], with r1 set to 0 by
>  the current function and r0 passed down from the caller. I'm not sure
>  if this is the exact same problem as the other tests, but feel free to
>  have a look at that code.
> >>>
> >>>
> >>> Have you tried running Asan-armhf-with-calls-Noinst-Test on the
> command line
> >>> without this patch applied? (It's possible that this is a pre-existing
> bug
> >>> in ARM varargs call lowering, and is unrelated to the bug we're trying
> to
> >>> track down. This code path performs an unrelated varargs call between a
> >>> va_start / va_end pair, which seems like quite a rare situation, and I
> could
> >>> easily believe there's something wrong with our lowering that allows
> some
> >>> portion of the outer va_list state to be clobbered in that scenario.)
> >>>
> >>
> >> It seems to work without the patch. I'm currently trying to reduce
> >> Asan-armhf-with-calls-Test and I'll get back to you with the IR or
> >> assembly files (whichever makes any difference).
> >>
>  Meanwhile, I've removed some clutter from Asan-armhf-with-calls-Test
>  (which is the original failure that we were seeing) and left only one
>  failing test that seemed small enough. I'll try to look at the
>  disassembly before/after the patch and maybe even run valgrind on it
>  (running it on the original binary naturally takes forever).
> 
>  Let me know if there's anything else I could try. I can also send you
>  disassembly or even LLVM IR for the Asan-armhf-with-calls-Noinst-Test
>  if you think it helps.
> 
>  Cheers,
>  Diana
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Diana Picus via cfe-commits]

Ugh, I'm not sure that last email made it through.

Here's the IR with -Xclang -disable-llvm-passes, at r310400 and r310401:
https://goo.gl/4n84SR
https://goo.gl/gxhxp9

The diff between them is small enough so I don't think there's much
point in reducing it further.
Hope that helps.

On 15 August 2017 at 17:51, Diana Picus  wrote:
> Actually, I'm not sure I need to reduce it, the diff between the IR
> with -Xclang -no-llvm-passes is pretty small for the version
> with/without the patch. See attached. I hope this helps, it does look
> like a problem with va_arg handling.
>
> Sorry again that this is going so slowly...
>
> On 15 August 2017 at 16:52, Diana Picus  wrote:
>> On 15 August 2017 at 01:25, Richard Smith  wrote:
>>> On 14 August 2017 at 03:27, Diana Picus via cfe-commits
>>>  wrote:

 Hi,

 Strangely enough, it turns out that if I run
 Asan-armhf-with-calls-Noinst-Test on the command line it fails,
 although it doesn't fail when run with lit.
>>>
>>>
>>> Looks like the crash is within the "use_colors == true" portion of
>>> ColoredPrintf, so this would make sense if running the test within lit turns
>>> off color support (perhaps because the output is not a terminal).
>>>
>>
>> That's a good catch, it seems if I run it with --gtest-color=no it
>> still passes, but prints some garbage at the end:
>> [ PASSED ] 97 tests.
>> YOU HAVE -1094542056 DISABLED F]�뀼��_8�pGD2
>>
>>

 I've attached the stack
 trace from gdb. It looks like some trouble passing down va_arg
 parameters, but I haven't looked into too much details. The segfault
 happens when we try to do a   ldrb   r3, [r0, r1], with r1 set to 0 by
 the current function and r0 passed down from the caller. I'm not sure
 if this is the exact same problem as the other tests, but feel free to
 have a look at that code.
>>>
>>>
>>> Have you tried running Asan-armhf-with-calls-Noinst-Test on the command line
>>> without this patch applied? (It's possible that this is a pre-existing bug
>>> in ARM varargs call lowering, and is unrelated to the bug we're trying to
>>> track down. This code path performs an unrelated varargs call between a
>>> va_start / va_end pair, which seems like quite a rare situation, and I could
>>> easily believe there's something wrong with our lowering that allows some
>>> portion of the outer va_list state to be clobbered in that scenario.)
>>>
>>
>> It seems to work without the patch. I'm currently trying to reduce
>> Asan-armhf-with-calls-Test and I'll get back to you with the IR or
>> assembly files (whichever makes any difference).
>>
 Meanwhile, I've removed some clutter from Asan-armhf-with-calls-Test
 (which is the original failure that we were seeing) and left only one
 failing test that seemed small enough. I'll try to look at the
 disassembly before/after the patch and maybe even run valgrind on it
 (running it on the original binary naturally takes forever).

 Let me know if there's anything else I could try. I can also send you
 disassembly or even LLVM IR for the Asan-armhf-with-calls-Noinst-Test
 if you think it helps.

 Cheers,
 Diana
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Diana Picus via cfe-commits]

On 15 August 2017 at 01:25, Richard Smith  wrote:
> On 14 August 2017 at 03:27, Diana Picus via cfe-commits
>  wrote:
>>
>> Hi,
>>
>> Strangely enough, it turns out that if I run
>> Asan-armhf-with-calls-Noinst-Test on the command line it fails,
>> although it doesn't fail when run with lit.
>
>
> Looks like the crash is within the "use_colors == true" portion of
> ColoredPrintf, so this would make sense if running the test within lit turns
> off color support (perhaps because the output is not a terminal).
>

That's a good catch, it seems if I run it with --gtest-color=no it
still passes, but prints some garbage at the end:
[ PASSED ] 97 tests.
YOU HAVE -1094542056 DISABLED F]�뀼��_8�pGD2


>>
>> I've attached the stack
>> trace from gdb. It looks like some trouble passing down va_arg
>> parameters, but I haven't looked into too much details. The segfault
>> happens when we try to do a   ldrb   r3, [r0, r1], with r1 set to 0 by
>> the current function and r0 passed down from the caller. I'm not sure
>> if this is the exact same problem as the other tests, but feel free to
>> have a look at that code.
>
>
> Have you tried running Asan-armhf-with-calls-Noinst-Test on the command line
> without this patch applied? (It's possible that this is a pre-existing bug
> in ARM varargs call lowering, and is unrelated to the bug we're trying to
> track down. This code path performs an unrelated varargs call between a
> va_start / va_end pair, which seems like quite a rare situation, and I could
> easily believe there's something wrong with our lowering that allows some
> portion of the outer va_list state to be clobbered in that scenario.)
>

It seems to work without the patch. I'm currently trying to reduce
Asan-armhf-with-calls-Test and I'll get back to you with the IR or
assembly files (whichever makes any difference).

>> Meanwhile, I've removed some clutter from Asan-armhf-with-calls-Test
>> (which is the original failure that we were seeing) and left only one
>> failing test that seemed small enough. I'll try to look at the
>> disassembly before/after the patch and maybe even run valgrind on it
>> (running it on the original binary naturally takes forever).
>>
>> Let me know if there's anything else I could try. I can also send you
>> disassembly or even LLVM IR for the Asan-armhf-with-calls-Noinst-Test
>> if you think it helps.
>>
>> Cheers,
>> Diana
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Richard Smith via cfe-commits]

On 14 August 2017 at 03:27, Diana Picus via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Hi,
>
> Strangely enough, it turns out that if I run
> Asan-armhf-with-calls-Noinst-Test on the command line it fails,
> although it doesn't fail when run with lit.


Looks like the crash is within the "use_colors == true" portion of
ColoredPrintf, so this would make sense if running the test within lit
turns off color support (perhaps because the output is not a terminal).


> I've attached the stack
> trace from gdb. It looks like some trouble passing down va_arg
> parameters, but I haven't looked into too much details. The segfault
> happens when we try to do a   ldrb   r3, [r0, r1], with r1 set to 0 by
> the current function and r0 passed down from the caller. I'm not sure
> if this is the exact same problem as the other tests, but feel free to
> have a look at that code.
>

Have you tried running Asan-armhf-with-calls-Noinst-Test on the command
line without this patch applied? (It's possible that this is a pre-existing
bug in ARM varargs call lowering, and is unrelated to the bug we're trying
to track down. This code path performs an unrelated varargs call between a
va_start / va_end pair, which seems like quite a rare situation, and I
could easily believe there's something wrong with our lowering that allows
some portion of the outer va_list state to be clobbered in that scenario.)

Meanwhile, I've removed some clutter from Asan-armhf-with-calls-Test
> (which is the original failure that we were seeing) and left only one
> failing test that seemed small enough. I'll try to look at the
> disassembly before/after the patch and maybe even run valgrind on it
> (running it on the original binary naturally takes forever).
>
> Let me know if there's anything else I could try. I can also send you
> disassembly or even LLVM IR for the Asan-armhf-with-calls-Noinst-Test
> if you think it helps.
>
> Cheers,
> Diana
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Vassil Vassilev via cfe-commits]

Hi,

  Ok that's a progress. Could you figure out which optimizer pass 
breaks it. I bet it is the inliner. I assume we could run a reducer on 
the ll file.


Cheers, Vassil
On 14/08/17 17:21, Diana Picus wrote:

Hi,

I didn't manage to reproduce this at -O0. Yes, I think the version in
1.8.0, slightly modified (see
llvm/utils/unittest/googletest/README.LLVM)

On 14 August 2017 at 17:06, Vassil Vassilev  wrote:

On 14/08/17 15:59, Diana Picus wrote:

No, the buildbots don't build with -O0 (at least not the ones that broke).

The command line for that particular object is:
build/./bin/clang -fPIC -fvisibility-inlines-hidden -Werror=date-time
-std=c++11 -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual
-Wmissing-field-initializers -pedantic -Wno-long-long
-Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor
-Wstring-conversion -fcolor-diagnostics -ffunction-sections
-fdata-sections -Wall -std=c++11 -Wno-unused-parameter
-Wno-unknown-warning-option -Wno-covered-switch-default
-DGTEST_NO_LLVM_RAW_OSTREAM=1 -DGTEST_HAS_RTTI=0
-I$LLVM_SRC/llvm/utils/unittest/googletest/include
-I/home/diana.picus/linaro-scripts/bisect/llvm/utils/unittest/googletest
-I$LLVM_SRC/llvm/projects/compiler-rt/include
-I$LLVM_SRC/llvm/projects/compiler-rt/lib
-I$LLVM_SRC/llvm/projects/compiler-rt/lib/asan
-I$LLVM_SRC/llvm/projects/compiler-rt/lib/sanitizer_common/tests
-fno-rtti -O2 -Wno-format -Werror=sign-compare -Wno-non-virtual-dtor
-Wno-variadic-macros -gline-tables-only -DASAN_HAS_BLACKLIST=1
-DASAN_HAS_EXCEPTIONS=1 -DASAN_UAR=0 -fsanitize=address

-fsanitize-blacklist=$LLVM_SRC/llvm/projects/compiler-rt/lib/asan/tests/asan_test.ignore
-mllvm -asan-instrumentation-with-call-threshold=0 -march=armv7-a
-mfloat-abi=hard -c -o
ASAN_INST_TEST_OBJECTS.asan_test.cc.armhf-with-calls.o
$LLVM_SRC/compiler-rt/lib/asan/tests/asan_test.cc

   Could you try to reproduce the issue with O0? This would rule out the
optimizer. Could you resend the O0 ll file, maybe I could find something
out. IIUC, the gtest version is 1.8.0?


Why would it contain gtest.cc? It seems to contain gtest.h and a bunch
of other internal gtest headers...


On 14 August 2017 at 16:51, Vassil Vassilev 
wrote:

On 14/08/17 15:08, Vassil Vassilev wrote:

On 14/08/17 13:04, Diana Picus wrote:

See attached.

Thanks! It looks like asan_test.i doesn't have gtest.cc which appears in
the stack trace. Am I missing something?

Could you paste the compiler invocation. Are we building with -O0 (I
see
quite a lot of things inline). Could it be an optimizer problem?


On 14 August 2017 at 13:30, Vassil Vassilev 
wrote:

On 14/08/17 11:27, Diana Picus wrote:

Hi,

Strangely enough, it turns out that if I run
Asan-armhf-with-calls-Noinst-Test on the command line it fails,
although it doesn't fail when run with lit. I've attached the stack
trace from gdb. It looks like some trouble passing down va_arg
parameters, but I haven't looked into too much details. The segfault
happens when we try to do a   ldrb   r3, [r0, r1], with r1 set to 0
by
the current function and r0 passed down from the caller. I'm not sure
if this is the exact same problem as the other tests, but feel free
to
have a look at that code.

Meanwhile, I've removed some clutter from Asan-armhf-with-calls-Test
(which is the original failure that we were seeing) and left only one
failing test that seemed small enough. I'll try to look at the
disassembly before/after the patch and maybe even run valgrind on it
(running it on the original binary naturally takes forever).

Let me know if there's anything else I could try. I can also send you
disassembly or even LLVM IR for the Asan-armhf-with-calls-Noinst-Test
if you think it helps.

 disassembly and LLVM will greatly help as well.


Cheers,
Diana

On 11 August 2017 at 15:34, Diana Picus 
wrote:

Well, these are ASAN tests, I'm not sure how that would interact
with
Valgrind.
Anyway, I'll try to reproduce the environment, I'm guessing it would
be best to catch this in gdb so I can actually see what's going on.

On 11 August 2017 at 15:21, Vassil Vassilev 
wrote:

That's really strange. It looks like some random behavior. Did you
run
some memory checker like valgrind?

Do the environment provided by the test runner and yours match?

Sent from my phone. Please excuse my brevity.


On 11 Aug 2017, at 15:58, Diana Picus 
wrote:

Hi again,

I finally got the debug build, but unfortunately the stack traces
that
the tests print look the same. My suspicion is that this is
because
the addresses printed by the tests are funny (i.e. odd numbers
instead
of divisible by 4). I tried to follow those addresses in an
objdump
of
the executable, but I didn't have much success since most of them
weren't really pointing to call instructions.

When I try to run the tests manually in the shell or in gdb, they
pass.

I'm not sure 

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Diana Picus via cfe-commits]

Hi,

I didn't manage to reproduce this at -O0. Yes, I think the version in
1.8.0, slightly modified (see
llvm/utils/unittest/googletest/README.LLVM)

On 14 August 2017 at 17:06, Vassil Vassilev  wrote:
> On 14/08/17 15:59, Diana Picus wrote:
>>
>> No, the buildbots don't build with -O0 (at least not the ones that broke).
>>
>> The command line for that particular object is:
>> build/./bin/clang -fPIC -fvisibility-inlines-hidden -Werror=date-time
>> -std=c++11 -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual
>> -Wmissing-field-initializers -pedantic -Wno-long-long
>> -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor
>> -Wstring-conversion -fcolor-diagnostics -ffunction-sections
>> -fdata-sections -Wall -std=c++11 -Wno-unused-parameter
>> -Wno-unknown-warning-option -Wno-covered-switch-default
>> -DGTEST_NO_LLVM_RAW_OSTREAM=1 -DGTEST_HAS_RTTI=0
>> -I$LLVM_SRC/llvm/utils/unittest/googletest/include
>> -I/home/diana.picus/linaro-scripts/bisect/llvm/utils/unittest/googletest
>> -I$LLVM_SRC/llvm/projects/compiler-rt/include
>> -I$LLVM_SRC/llvm/projects/compiler-rt/lib
>> -I$LLVM_SRC/llvm/projects/compiler-rt/lib/asan
>> -I$LLVM_SRC/llvm/projects/compiler-rt/lib/sanitizer_common/tests
>> -fno-rtti -O2 -Wno-format -Werror=sign-compare -Wno-non-virtual-dtor
>> -Wno-variadic-macros -gline-tables-only -DASAN_HAS_BLACKLIST=1
>> -DASAN_HAS_EXCEPTIONS=1 -DASAN_UAR=0 -fsanitize=address
>>
>> -fsanitize-blacklist=$LLVM_SRC/llvm/projects/compiler-rt/lib/asan/tests/asan_test.ignore
>> -mllvm -asan-instrumentation-with-call-threshold=0 -march=armv7-a
>> -mfloat-abi=hard -c -o
>> ASAN_INST_TEST_OBJECTS.asan_test.cc.armhf-with-calls.o
>> $LLVM_SRC/compiler-rt/lib/asan/tests/asan_test.cc
>
>   Could you try to reproduce the issue with O0? This would rule out the
> optimizer. Could you resend the O0 ll file, maybe I could find something
> out. IIUC, the gtest version is 1.8.0?
>
>>
>> Why would it contain gtest.cc? It seems to contain gtest.h and a bunch
>> of other internal gtest headers...
>>
>>
>> On 14 August 2017 at 16:51, Vassil Vassilev 
>> wrote:
>>>
>>> On 14/08/17 15:08, Vassil Vassilev wrote:

 On 14/08/17 13:04, Diana Picus wrote:
>
> See attached.

 Thanks! It looks like asan_test.i doesn't have gtest.cc which appears in
 the stack trace. Am I missing something?
>>>
>>>Could you paste the compiler invocation. Are we building with -O0 (I
>>> see
>>> quite a lot of things inline). Could it be an optimizer problem?
>>>
> On 14 August 2017 at 13:30, Vassil Vassilev 
> wrote:
>>
>> On 14/08/17 11:27, Diana Picus wrote:
>>>
>>> Hi,
>>>
>>> Strangely enough, it turns out that if I run
>>> Asan-armhf-with-calls-Noinst-Test on the command line it fails,
>>> although it doesn't fail when run with lit. I've attached the stack
>>> trace from gdb. It looks like some trouble passing down va_arg
>>> parameters, but I haven't looked into too much details. The segfault
>>> happens when we try to do a   ldrb   r3, [r0, r1], with r1 set to 0
>>> by
>>> the current function and r0 passed down from the caller. I'm not sure
>>> if this is the exact same problem as the other tests, but feel free
>>> to
>>> have a look at that code.
>>>
>>> Meanwhile, I've removed some clutter from Asan-armhf-with-calls-Test
>>> (which is the original failure that we were seeing) and left only one
>>> failing test that seemed small enough. I'll try to look at the
>>> disassembly before/after the patch and maybe even run valgrind on it
>>> (running it on the original binary naturally takes forever).
>>>
>>> Let me know if there's anything else I could try. I can also send you
>>> disassembly or even LLVM IR for the Asan-armhf-with-calls-Noinst-Test
>>> if you think it helps.
>>
>> disassembly and LLVM will greatly help as well.
>>
>>> Cheers,
>>> Diana
>>>
>>> On 11 August 2017 at 15:34, Diana Picus 
>>> wrote:

 Well, these are ASAN tests, I'm not sure how that would interact
 with
 Valgrind.
 Anyway, I'll try to reproduce the environment, I'm guessing it would
 be best to catch this in gdb so I can actually see what's going on.

 On 11 August 2017 at 15:21, Vassil Vassilev 
 wrote:
>
> That's really strange. It looks like some random behavior. Did you
> run
> some memory checker like valgrind?
>
> Do the environment provided by the test runner and yours match?
>
> Sent from my phone. Please excuse my brevity.
>
>> On 11 Aug 2017, at 15:58, Diana Picus 
>> wrote:
>>
>> Hi again,
>>
>> I finally got the debug build, but 

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Vassil Vassilev via cfe-commits]

On 14/08/17 15:59, Diana Picus wrote:

No, the buildbots don't build with -O0 (at least not the ones that broke).

The command line for that particular object is:
build/./bin/clang -fPIC -fvisibility-inlines-hidden -Werror=date-time
-std=c++11 -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual
-Wmissing-field-initializers -pedantic -Wno-long-long
-Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor
-Wstring-conversion -fcolor-diagnostics -ffunction-sections
-fdata-sections -Wall -std=c++11 -Wno-unused-parameter
-Wno-unknown-warning-option -Wno-covered-switch-default
-DGTEST_NO_LLVM_RAW_OSTREAM=1 -DGTEST_HAS_RTTI=0
-I$LLVM_SRC/llvm/utils/unittest/googletest/include
-I/home/diana.picus/linaro-scripts/bisect/llvm/utils/unittest/googletest
-I$LLVM_SRC/llvm/projects/compiler-rt/include
-I$LLVM_SRC/llvm/projects/compiler-rt/lib
-I$LLVM_SRC/llvm/projects/compiler-rt/lib/asan
-I$LLVM_SRC/llvm/projects/compiler-rt/lib/sanitizer_common/tests
-fno-rtti -O2 -Wno-format -Werror=sign-compare -Wno-non-virtual-dtor
-Wno-variadic-macros -gline-tables-only -DASAN_HAS_BLACKLIST=1
-DASAN_HAS_EXCEPTIONS=1 -DASAN_UAR=0 -fsanitize=address
-fsanitize-blacklist=$LLVM_SRC/llvm/projects/compiler-rt/lib/asan/tests/asan_test.ignore
-mllvm -asan-instrumentation-with-call-threshold=0 -march=armv7-a
-mfloat-abi=hard -c -o
ASAN_INST_TEST_OBJECTS.asan_test.cc.armhf-with-calls.o
$LLVM_SRC/compiler-rt/lib/asan/tests/asan_test.cc
  Could you try to reproduce the issue with O0? This would rule out the 
optimizer. Could you resend the O0 ll file, maybe I could find something 
out. IIUC, the gtest version is 1.8.0?


Why would it contain gtest.cc? It seems to contain gtest.h and a bunch
of other internal gtest headers...


On 14 August 2017 at 16:51, Vassil Vassilev  wrote:

On 14/08/17 15:08, Vassil Vassilev wrote:

On 14/08/17 13:04, Diana Picus wrote:

See attached.

Thanks! It looks like asan_test.i doesn't have gtest.cc which appears in
the stack trace. Am I missing something?

   Could you paste the compiler invocation. Are we building with -O0 (I see
quite a lot of things inline). Could it be an optimizer problem?


On 14 August 2017 at 13:30, Vassil Vassilev 
wrote:

On 14/08/17 11:27, Diana Picus wrote:

Hi,

Strangely enough, it turns out that if I run
Asan-armhf-with-calls-Noinst-Test on the command line it fails,
although it doesn't fail when run with lit. I've attached the stack
trace from gdb. It looks like some trouble passing down va_arg
parameters, but I haven't looked into too much details. The segfault
happens when we try to do a   ldrb   r3, [r0, r1], with r1 set to 0 by
the current function and r0 passed down from the caller. I'm not sure
if this is the exact same problem as the other tests, but feel free to
have a look at that code.

Meanwhile, I've removed some clutter from Asan-armhf-with-calls-Test
(which is the original failure that we were seeing) and left only one
failing test that seemed small enough. I'll try to look at the
disassembly before/after the patch and maybe even run valgrind on it
(running it on the original binary naturally takes forever).

Let me know if there's anything else I could try. I can also send you
disassembly or even LLVM IR for the Asan-armhf-with-calls-Noinst-Test
if you think it helps.

disassembly and LLVM will greatly help as well.


Cheers,
Diana

On 11 August 2017 at 15:34, Diana Picus  wrote:

Well, these are ASAN tests, I'm not sure how that would interact with
Valgrind.
Anyway, I'll try to reproduce the environment, I'm guessing it would
be best to catch this in gdb so I can actually see what's going on.

On 11 August 2017 at 15:21, Vassil Vassilev 
wrote:

That's really strange. It looks like some random behavior. Did you
run
some memory checker like valgrind?

Do the environment provided by the test runner and yours match?

Sent from my phone. Please excuse my brevity.


On 11 Aug 2017, at 15:58, Diana Picus 
wrote:

Hi again,

I finally got the debug build, but unfortunately the stack traces
that
the tests print look the same. My suspicion is that this is because
the addresses printed by the tests are funny (i.e. odd numbers
instead
of divisible by 4). I tried to follow those addresses in an objdump
of
the executable, but I didn't have much success since most of them
weren't really pointing to call instructions.

When I try to run the tests manually in the shell or in gdb, they
pass.

I'm not sure what else to try. Thoughts?

Thanks,
Diana


On 11 August 2017 at 11:14, Diana Picus 
wrote:
Hi guys,

I'm SO sorry about the delays. I've been having all sorts of
trouble
getting that debug build on the board (from ld running out of
memory
to the board just crashing on me, in which case I need to ask
someone
else to reboot it because I can't power cycle it remotely). I can
assure you this is one of my top 

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Diana Picus via cfe-commits]

On 14 August 2017 at 16:59, Diana Picus  wrote:
> No, the buildbots don't build with -O0 (at least not the ones that broke).
>
> The command line for that particular object is:
> build/./bin/clang -fPIC -fvisibility-inlines-hidden -Werror=date-time
> -std=c++11 -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual
> -Wmissing-field-initializers -pedantic -Wno-long-long
> -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor
> -Wstring-conversion -fcolor-diagnostics -ffunction-sections
> -fdata-sections -Wall -std=c++11 -Wno-unused-parameter
> -Wno-unknown-warning-option -Wno-covered-switch-default
> -DGTEST_NO_LLVM_RAW_OSTREAM=1 -DGTEST_HAS_RTTI=0
> -I$LLVM_SRC/llvm/utils/unittest/googletest/include
> -I/home/diana.picus/linaro-scripts/bisect/llvm/utils/unittest/googletest
> -I$LLVM_SRC/llvm/projects/compiler-rt/include
> -I$LLVM_SRC/llvm/projects/compiler-rt/lib
> -I$LLVM_SRC/llvm/projects/compiler-rt/lib/asan
> -I$LLVM_SRC/llvm/projects/compiler-rt/lib/sanitizer_common/tests
> -fno-rtti -O2 -Wno-format -Werror=sign-compare -Wno-non-virtual-dtor
> -Wno-variadic-macros -gline-tables-only -DASAN_HAS_BLACKLIST=1
> -DASAN_HAS_EXCEPTIONS=1 -DASAN_UAR=0 -fsanitize=address
> -fsanitize-blacklist=$LLVM_SRC/llvm/projects/compiler-rt/lib/asan/tests/asan_test.ignore
> -mllvm -asan-instrumentation-with-call-threshold=0 -march=armv7-a
> -mfloat-abi=hard -c -o
> ASAN_INST_TEST_OBJECTS.asan_test.cc.armhf-with-calls.o
> $LLVM_SRC/compiler-rt/lib/asan/tests/asan_test.cc
>
> Why would it contain gtest.cc? It seems to contain gtest.h and a bunch
> of other internal gtest headers...
>

In particular, gtest.cc is included into gtest_all.cc, which compiled
to ASAN_INST_TEST_OBJECTS.gtest_all.cc.armhf-with-calls.o and then
added to the link line, which looks like this:
build/./bin/clang
ASAN_INST_TEST_OBJECTS.gtest-all.cc.armhf-with-calls.o
ASAN_INST_TEST_OBJECTS.asan_asm_test.cc.armhf-with-calls.o
ASAN_INST_TEST_OBJECTS.asan_globals_test.cc.armhf-with-calls.o
ASAN_INST_TEST_OBJECTS.asan_interface_test.cc.armhf-with-calls.o
ASAN_INST_TEST_OBJECTS.asan_internal_interface_test.cc.armhf-with-calls.o
ASAN_INST_TEST_OBJECTS.asan_test.cc.armhf-with-calls.o
ASAN_INST_TEST_OBJECTS.asan_oob_test.cc.armhf-with-calls.o
ASAN_INST_TEST_OBJECTS.asan_mem_test.cc.armhf-with-calls.o
ASAN_INST_TEST_OBJECTS.asan_str_test.cc.armhf-with-calls.o
ASAN_INST_TEST_OBJECTS.asan_test_main.cc.armhf-with-calls.o -o
build/projects/compiler-rt/lib/asan/tests/default/Asan-armhf-with-calls-Test
-Wl,-allow-shlib-undefined -g --driver-mode=g++ -fsanitize=address
-march=armv7-a -mfloat-abi=hard

>
> On 14 August 2017 at 16:51, Vassil Vassilev  wrote:
>> On 14/08/17 15:08, Vassil Vassilev wrote:
>>>
>>> On 14/08/17 13:04, Diana Picus wrote:

 See attached.
>>>
>>> Thanks! It looks like asan_test.i doesn't have gtest.cc which appears in
>>> the stack trace. Am I missing something?
>>
>>   Could you paste the compiler invocation. Are we building with -O0 (I see
>> quite a lot of things inline). Could it be an optimizer problem?
>>

 On 14 August 2017 at 13:30, Vassil Vassilev 
 wrote:
>
> On 14/08/17 11:27, Diana Picus wrote:
>>
>> Hi,
>>
>> Strangely enough, it turns out that if I run
>> Asan-armhf-with-calls-Noinst-Test on the command line it fails,
>> although it doesn't fail when run with lit. I've attached the stack
>> trace from gdb. It looks like some trouble passing down va_arg
>> parameters, but I haven't looked into too much details. The segfault
>> happens when we try to do a   ldrb   r3, [r0, r1], with r1 set to 0 by
>> the current function and r0 passed down from the caller. I'm not sure
>> if this is the exact same problem as the other tests, but feel free to
>> have a look at that code.
>>
>> Meanwhile, I've removed some clutter from Asan-armhf-with-calls-Test
>> (which is the original failure that we were seeing) and left only one
>> failing test that seemed small enough. I'll try to look at the
>> disassembly before/after the patch and maybe even run valgrind on it
>> (running it on the original binary naturally takes forever).
>>
>> Let me know if there's anything else I could try. I can also send you
>> disassembly or even LLVM IR for the Asan-armhf-with-calls-Noinst-Test
>> if you think it helps.
>
>disassembly and LLVM will greatly help as well.
>
>> Cheers,
>> Diana
>>
>> On 11 August 2017 at 15:34, Diana Picus  wrote:
>>>
>>> Well, these are ASAN tests, I'm not sure how that would interact with
>>> Valgrind.
>>> Anyway, I'll try to reproduce the environment, I'm guessing it would
>>> be best to catch this in gdb so I can actually see what's going on.
>>>
>>> On 11 August 2017 at 15:21, Vassil Vassilev 
>>> wrote:

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Diana Picus via cfe-commits]

No, the buildbots don't build with -O0 (at least not the ones that broke).

The command line for that particular object is:
build/./bin/clang -fPIC -fvisibility-inlines-hidden -Werror=date-time
-std=c++11 -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual
-Wmissing-field-initializers -pedantic -Wno-long-long
-Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor
-Wstring-conversion -fcolor-diagnostics -ffunction-sections
-fdata-sections -Wall -std=c++11 -Wno-unused-parameter
-Wno-unknown-warning-option -Wno-covered-switch-default
-DGTEST_NO_LLVM_RAW_OSTREAM=1 -DGTEST_HAS_RTTI=0
-I$LLVM_SRC/llvm/utils/unittest/googletest/include
-I/home/diana.picus/linaro-scripts/bisect/llvm/utils/unittest/googletest
-I$LLVM_SRC/llvm/projects/compiler-rt/include
-I$LLVM_SRC/llvm/projects/compiler-rt/lib
-I$LLVM_SRC/llvm/projects/compiler-rt/lib/asan
-I$LLVM_SRC/llvm/projects/compiler-rt/lib/sanitizer_common/tests
-fno-rtti -O2 -Wno-format -Werror=sign-compare -Wno-non-virtual-dtor
-Wno-variadic-macros -gline-tables-only -DASAN_HAS_BLACKLIST=1
-DASAN_HAS_EXCEPTIONS=1 -DASAN_UAR=0 -fsanitize=address
-fsanitize-blacklist=$LLVM_SRC/llvm/projects/compiler-rt/lib/asan/tests/asan_test.ignore
-mllvm -asan-instrumentation-with-call-threshold=0 -march=armv7-a
-mfloat-abi=hard -c -o
ASAN_INST_TEST_OBJECTS.asan_test.cc.armhf-with-calls.o
$LLVM_SRC/compiler-rt/lib/asan/tests/asan_test.cc

Why would it contain gtest.cc? It seems to contain gtest.h and a bunch
of other internal gtest headers...


On 14 August 2017 at 16:51, Vassil Vassilev  wrote:
> On 14/08/17 15:08, Vassil Vassilev wrote:
>>
>> On 14/08/17 13:04, Diana Picus wrote:
>>>
>>> See attached.
>>
>> Thanks! It looks like asan_test.i doesn't have gtest.cc which appears in
>> the stack trace. Am I missing something?
>
>   Could you paste the compiler invocation. Are we building with -O0 (I see
> quite a lot of things inline). Could it be an optimizer problem?
>
>>>
>>> On 14 August 2017 at 13:30, Vassil Vassilev 
>>> wrote:

 On 14/08/17 11:27, Diana Picus wrote:
>
> Hi,
>
> Strangely enough, it turns out that if I run
> Asan-armhf-with-calls-Noinst-Test on the command line it fails,
> although it doesn't fail when run with lit. I've attached the stack
> trace from gdb. It looks like some trouble passing down va_arg
> parameters, but I haven't looked into too much details. The segfault
> happens when we try to do a   ldrb   r3, [r0, r1], with r1 set to 0 by
> the current function and r0 passed down from the caller. I'm not sure
> if this is the exact same problem as the other tests, but feel free to
> have a look at that code.
>
> Meanwhile, I've removed some clutter from Asan-armhf-with-calls-Test
> (which is the original failure that we were seeing) and left only one
> failing test that seemed small enough. I'll try to look at the
> disassembly before/after the patch and maybe even run valgrind on it
> (running it on the original binary naturally takes forever).
>
> Let me know if there's anything else I could try. I can also send you
> disassembly or even LLVM IR for the Asan-armhf-with-calls-Noinst-Test
> if you think it helps.

disassembly and LLVM will greatly help as well.

> Cheers,
> Diana
>
> On 11 August 2017 at 15:34, Diana Picus  wrote:
>>
>> Well, these are ASAN tests, I'm not sure how that would interact with
>> Valgrind.
>> Anyway, I'll try to reproduce the environment, I'm guessing it would
>> be best to catch this in gdb so I can actually see what's going on.
>>
>> On 11 August 2017 at 15:21, Vassil Vassilev 
>> wrote:
>>>
>>> That's really strange. It looks like some random behavior. Did you
>>> run
>>> some memory checker like valgrind?
>>>
>>> Do the environment provided by the test runner and yours match?
>>>
>>> Sent from my phone. Please excuse my brevity.
>>>
 On 11 Aug 2017, at 15:58, Diana Picus 
 wrote:

 Hi again,

 I finally got the debug build, but unfortunately the stack traces
 that
 the tests print look the same. My suspicion is that this is because
 the addresses printed by the tests are funny (i.e. odd numbers
 instead
 of divisible by 4). I tried to follow those addresses in an objdump
 of
 the executable, but I didn't have much success since most of them
 weren't really pointing to call instructions.

 When I try to run the tests manually in the shell or in gdb, they
 pass.

 I'm not sure what else to try. Thoughts?

 Thanks,
 Diana

> On 11 August 2017 at 11:14, Diana Picus 
> wrote:

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Vassil Vassilev via cfe-commits]

On 14/08/17 15:08, Vassil Vassilev wrote:

On 14/08/17 13:04, Diana Picus wrote:

See attached.
Thanks! It looks like asan_test.i doesn't have gtest.cc which appears 
in the stack trace. Am I missing something?
  Could you paste the compiler invocation. Are we building with -O0 (I 
see quite a lot of things inline). Could it be an optimizer problem?


On 14 August 2017 at 13:30, Vassil Vassilev  
wrote:

On 14/08/17 11:27, Diana Picus wrote:

Hi,

Strangely enough, it turns out that if I run
Asan-armhf-with-calls-Noinst-Test on the command line it fails,
although it doesn't fail when run with lit. I've attached the stack
trace from gdb. It looks like some trouble passing down va_arg
parameters, but I haven't looked into too much details. The segfault
happens when we try to do a   ldrb   r3, [r0, r1], with r1 set to 0 by
the current function and r0 passed down from the caller. I'm not sure
if this is the exact same problem as the other tests, but feel free to
have a look at that code.

Meanwhile, I've removed some clutter from Asan-armhf-with-calls-Test
(which is the original failure that we were seeing) and left only one
failing test that seemed small enough. I'll try to look at the
disassembly before/after the patch and maybe even run valgrind on it
(running it on the original binary naturally takes forever).

Let me know if there's anything else I could try. I can also send you
disassembly or even LLVM IR for the Asan-armhf-with-calls-Noinst-Test
if you think it helps.

   disassembly and LLVM will greatly help as well.


Cheers,
Diana

On 11 August 2017 at 15:34, Diana Picus  
wrote:

Well, these are ASAN tests, I'm not sure how that would interact with
Valgrind.
Anyway, I'll try to reproduce the environment, I'm guessing it would
be best to catch this in gdb so I can actually see what's going on.

On 11 August 2017 at 15:21, Vassil Vassilev 
wrote:
That's really strange. It looks like some random behavior. Did 
you run

some memory checker like valgrind?

Do the environment provided by the test runner and yours match?

Sent from my phone. Please excuse my brevity.

On 11 Aug 2017, at 15:58, Diana Picus  
wrote:


Hi again,

I finally got the debug build, but unfortunately the stack 
traces that

the tests print look the same. My suspicion is that this is because
the addresses printed by the tests are funny (i.e. odd numbers 
instead
of divisible by 4). I tried to follow those addresses in an 
objdump of

the executable, but I didn't have much success since most of them
weren't really pointing to call instructions.

When I try to run the tests manually in the shell or in gdb, 
they pass.


I'm not sure what else to try. Thoughts?

Thanks,
Diana


On 11 August 2017 at 11:14, Diana Picus 
wrote:
Hi guys,

I'm SO sorry about the delays. I've been having all sorts of 
trouble
getting that debug build on the board (from ld running out of 
memory
to the board just crashing on me, in which case I need to ask 
someone

else to reboot it because I can't power cycle it remotely). I can
assure you this is one of my top priorities, I'll get those stack
traces as soon as I can.

Thanks for your patience and sorry again,
Diana


On 10 August 2017 at 22:55, Richard Smith 
wrote:
Any news on this? We want this change in Clang 5, so the 
sooner we

can
understand and fix this regression the better...

On 10 August 2017 at 01:28, Diana Picus via cfe-commits
 wrote:

Hi Vassil,

My build is in progress, but since it's a full build it's 
probably
going to take another couple of hours to complete. I'll let 
you know

when it's done.

Thanks,
Diana

On 10 August 2017 at 10:09, Vassil Vassilev 


wrote:

It looks like I can not reproduce it on osx (non-arm)... :(

On 09/08/17 22:54, Diana Picus wrote:

Reverting this also fixed the selfhost bots:



http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15-full-sh/builds/2142 





http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost/builds/2309 





http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost-neon/builds/1819 



I'm afraid the logs for those look even less helpful.

On 9 August 2017 at 16:17, Diana Picus 


wrote:

Hi,

See attached. FWIW, when I ran this on a very similar 
machine, I

got
194 failures, all of which went away after reverting. So 
there

might
be something fishy going on.

Regards,
Diana

On 9 August 2017 at 15:02, Vassil Vassilev

wrote:

Hi Diana,

It seems the service is down. Could you send us the 
details

of the
failures (incl stack traces if any)

Many thanks,
Vassil


On 09/08/17 15:27, Diana Picus via cfe-commits wrote:

Hi Richard,

I'm sorry but I've reverted this in r310464 because it was
breaking
some ASAN tests on this bot:

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Vassil Vassilev via cfe-commits]

On 14/08/17 13:04, Diana Picus wrote:

See attached.
Thanks! It looks like asan_test.i doesn't have gtest.cc which appears in 
the stack trace. Am I missing something?


On 14 August 2017 at 13:30, Vassil Vassilev  wrote:

On 14/08/17 11:27, Diana Picus wrote:

Hi,

Strangely enough, it turns out that if I run
Asan-armhf-with-calls-Noinst-Test on the command line it fails,
although it doesn't fail when run with lit. I've attached the stack
trace from gdb. It looks like some trouble passing down va_arg
parameters, but I haven't looked into too much details. The segfault
happens when we try to do a   ldrb   r3, [r0, r1], with r1 set to 0 by
the current function and r0 passed down from the caller. I'm not sure
if this is the exact same problem as the other tests, but feel free to
have a look at that code.

Meanwhile, I've removed some clutter from Asan-armhf-with-calls-Test
(which is the original failure that we were seeing) and left only one
failing test that seemed small enough. I'll try to look at the
disassembly before/after the patch and maybe even run valgrind on it
(running it on the original binary naturally takes forever).

Let me know if there's anything else I could try. I can also send you
disassembly or even LLVM IR for the Asan-armhf-with-calls-Noinst-Test
if you think it helps.

   disassembly and LLVM will greatly help as well.


Cheers,
Diana

On 11 August 2017 at 15:34, Diana Picus  wrote:

Well, these are ASAN tests, I'm not sure how that would interact with
Valgrind.
Anyway, I'll try to reproduce the environment, I'm guessing it would
be best to catch this in gdb so I can actually see what's going on.

On 11 August 2017 at 15:21, Vassil Vassilev 
wrote:

That's really strange. It looks like some random behavior. Did you run
some memory checker like valgrind?

Do the environment provided by the test runner and yours match?

Sent from my phone. Please excuse my brevity.


On 11 Aug 2017, at 15:58, Diana Picus  wrote:

Hi again,

I finally got the debug build, but unfortunately the stack traces that
the tests print look the same. My suspicion is that this is because
the addresses printed by the tests are funny (i.e. odd numbers instead
of divisible by 4). I tried to follow those addresses in an objdump of
the executable, but I didn't have much success since most of them
weren't really pointing to call instructions.

When I try to run the tests manually in the shell or in gdb, they pass.

I'm not sure what else to try. Thoughts?

Thanks,
Diana


On 11 August 2017 at 11:14, Diana Picus 
wrote:
Hi guys,

I'm SO sorry about the delays. I've been having all sorts of trouble
getting that debug build on the board (from ld running out of memory
to the board just crashing on me, in which case I need to ask someone
else to reboot it because I can't power cycle it remotely). I can
assure you this is one of my top priorities, I'll get those stack
traces as soon as I can.

Thanks for your patience and sorry again,
Diana


On 10 August 2017 at 22:55, Richard Smith 
wrote:
Any news on this? We want this change in Clang 5, so the sooner we
can
understand and fix this regression the better...

On 10 August 2017 at 01:28, Diana Picus via cfe-commits
 wrote:

Hi Vassil,

My build is in progress, but since it's a full build it's probably
going to take another couple of hours to complete. I'll let you know
when it's done.

Thanks,
Diana

On 10 August 2017 at 10:09, Vassil Vassilev 
wrote:

It looks like I can not reproduce it on osx (non-arm)... :(

On 09/08/17 22:54, Diana Picus wrote:

Reverting this also fixed the selfhost bots:



http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15-full-sh/builds/2142



http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost/builds/2309



http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost-neon/builds/1819

I'm afraid the logs for those look even less helpful.


On 9 August 2017 at 16:17, Diana Picus 
wrote:

Hi,

See attached. FWIW, when I ran this on a very similar machine, I
got
194 failures, all of which went away after reverting. So there
might
be something fishy going on.

Regards,
Diana

On 9 August 2017 at 15:02, Vassil Vassilev

wrote:

Hi Diana,

It seems the service is down. Could you send us the details
of the
failures (incl stack traces if any)

Many thanks,
Vassil


On 09/08/17 15:27, Diana Picus via cfe-commits wrote:

Hi Richard,

I'm sorry but I've reverted this in r310464 because it was
breaking
some ASAN tests on this bot:



http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/9452

Please let me know if I can help debug this.

Cheers,
Diana

On 8 August 2017 at 21:14, Richard Smith via cfe-commits
 wrote:

I forgot to say:

Based on a 

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Diana Picus via cfe-commits]

Also if you want the disassembly for the whole test executable (with
just that test in it):
https://goo.gl/pjULbN

It's 177MB though.

On 14 August 2017 at 14:04, Diana Picus  wrote:
> See attached.
>
> On 14 August 2017 at 13:30, Vassil Vassilev  wrote:
>> On 14/08/17 11:27, Diana Picus wrote:
>>>
>>> Hi,
>>>
>>> Strangely enough, it turns out that if I run
>>> Asan-armhf-with-calls-Noinst-Test on the command line it fails,
>>> although it doesn't fail when run with lit. I've attached the stack
>>> trace from gdb. It looks like some trouble passing down va_arg
>>> parameters, but I haven't looked into too much details. The segfault
>>> happens when we try to do a   ldrb   r3, [r0, r1], with r1 set to 0 by
>>> the current function and r0 passed down from the caller. I'm not sure
>>> if this is the exact same problem as the other tests, but feel free to
>>> have a look at that code.
>>>
>>> Meanwhile, I've removed some clutter from Asan-armhf-with-calls-Test
>>> (which is the original failure that we were seeing) and left only one
>>> failing test that seemed small enough. I'll try to look at the
>>> disassembly before/after the patch and maybe even run valgrind on it
>>> (running it on the original binary naturally takes forever).
>>>
>>> Let me know if there's anything else I could try. I can also send you
>>> disassembly or even LLVM IR for the Asan-armhf-with-calls-Noinst-Test
>>> if you think it helps.
>>
>>   disassembly and LLVM will greatly help as well.
>>
>>>
>>> Cheers,
>>> Diana
>>>
>>> On 11 August 2017 at 15:34, Diana Picus  wrote:

 Well, these are ASAN tests, I'm not sure how that would interact with
 Valgrind.
 Anyway, I'll try to reproduce the environment, I'm guessing it would
 be best to catch this in gdb so I can actually see what's going on.

 On 11 August 2017 at 15:21, Vassil Vassilev 
 wrote:
>
> That's really strange. It looks like some random behavior. Did you run
> some memory checker like valgrind?
>
> Do the environment provided by the test runner and yours match?
>
> Sent from my phone. Please excuse my brevity.
>
>> On 11 Aug 2017, at 15:58, Diana Picus  wrote:
>>
>> Hi again,
>>
>> I finally got the debug build, but unfortunately the stack traces that
>> the tests print look the same. My suspicion is that this is because
>> the addresses printed by the tests are funny (i.e. odd numbers instead
>> of divisible by 4). I tried to follow those addresses in an objdump of
>> the executable, but I didn't have much success since most of them
>> weren't really pointing to call instructions.
>>
>> When I try to run the tests manually in the shell or in gdb, they pass.
>>
>> I'm not sure what else to try. Thoughts?
>>
>> Thanks,
>> Diana
>>
>>> On 11 August 2017 at 11:14, Diana Picus 
>>> wrote:
>>> Hi guys,
>>>
>>> I'm SO sorry about the delays. I've been having all sorts of trouble
>>> getting that debug build on the board (from ld running out of memory
>>> to the board just crashing on me, in which case I need to ask someone
>>> else to reboot it because I can't power cycle it remotely). I can
>>> assure you this is one of my top priorities, I'll get those stack
>>> traces as soon as I can.
>>>
>>> Thanks for your patience and sorry again,
>>> Diana
>>>
 On 10 August 2017 at 22:55, Richard Smith 
 wrote:
 Any news on this? We want this change in Clang 5, so the sooner we
 can
 understand and fix this regression the better...

 On 10 August 2017 at 01:28, Diana Picus via cfe-commits
  wrote:
>
> Hi Vassil,
>
> My build is in progress, but since it's a full build it's probably
> going to take another couple of hours to complete. I'll let you know
> when it's done.
>
> Thanks,
> Diana
>
> On 10 August 2017 at 10:09, Vassil Vassilev 
> wrote:
>>
>> It looks like I can not reproduce it on osx (non-arm)... :(
>>>
>>> On 09/08/17 22:54, Diana Picus wrote:
>>>
>>> Reverting this also fixed the selfhost bots:
>>>
>>>
>>>
>>> http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15-full-sh/builds/2142
>>>
>>>
>>>
>>> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost/builds/2309
>>>
>>>
>>>
>>> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost-neon/builds/1819
>>>
>>> I'm afraid the logs for those look even less helpful.
>>>
 

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Vassil Vassilev via cfe-commits]

On 14/08/17 11:27, Diana Picus wrote:

Hi,

Strangely enough, it turns out that if I run
Asan-armhf-with-calls-Noinst-Test on the command line it fails,
although it doesn't fail when run with lit. I've attached the stack
trace from gdb. It looks like some trouble passing down va_arg
parameters, but I haven't looked into too much details. The segfault
happens when we try to do a   ldrb   r3, [r0, r1], with r1 set to 0 by
the current function and r0 passed down from the caller. I'm not sure
if this is the exact same problem as the other tests, but feel free to
have a look at that code.

Meanwhile, I've removed some clutter from Asan-armhf-with-calls-Test
(which is the original failure that we were seeing) and left only one
failing test that seemed small enough. I'll try to look at the
disassembly before/after the patch and maybe even run valgrind on it
(running it on the original binary naturally takes forever).

Let me know if there's anything else I could try. I can also send you
disassembly or even LLVM IR for the Asan-armhf-with-calls-Noinst-Test
if you think it helps.

  disassembly and LLVM will greatly help as well.


Cheers,
Diana

On 11 August 2017 at 15:34, Diana Picus  wrote:

Well, these are ASAN tests, I'm not sure how that would interact with Valgrind.
Anyway, I'll try to reproduce the environment, I'm guessing it would
be best to catch this in gdb so I can actually see what's going on.

On 11 August 2017 at 15:21, Vassil Vassilev  wrote:

That's really strange. It looks like some random behavior. Did you run some 
memory checker like valgrind?

Do the environment provided by the test runner and yours match?

Sent from my phone. Please excuse my brevity.


On 11 Aug 2017, at 15:58, Diana Picus  wrote:

Hi again,

I finally got the debug build, but unfortunately the stack traces that
the tests print look the same. My suspicion is that this is because
the addresses printed by the tests are funny (i.e. odd numbers instead
of divisible by 4). I tried to follow those addresses in an objdump of
the executable, but I didn't have much success since most of them
weren't really pointing to call instructions.

When I try to run the tests manually in the shell or in gdb, they pass.

I'm not sure what else to try. Thoughts?

Thanks,
Diana


On 11 August 2017 at 11:14, Diana Picus  wrote:
Hi guys,

I'm SO sorry about the delays. I've been having all sorts of trouble
getting that debug build on the board (from ld running out of memory
to the board just crashing on me, in which case I need to ask someone
else to reboot it because I can't power cycle it remotely). I can
assure you this is one of my top priorities, I'll get those stack
traces as soon as I can.

Thanks for your patience and sorry again,
Diana


On 10 August 2017 at 22:55, Richard Smith  wrote:
Any news on this? We want this change in Clang 5, so the sooner we can
understand and fix this regression the better...

On 10 August 2017 at 01:28, Diana Picus via cfe-commits
 wrote:

Hi Vassil,

My build is in progress, but since it's a full build it's probably
going to take another couple of hours to complete. I'll let you know
when it's done.

Thanks,
Diana

On 10 August 2017 at 10:09, Vassil Vassilev 
wrote:

It looks like I can not reproduce it on osx (non-arm)... :(

On 09/08/17 22:54, Diana Picus wrote:

Reverting this also fixed the selfhost bots:


http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15-full-sh/builds/2142


http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost/builds/2309


http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost-neon/builds/1819

I'm afraid the logs for those look even less helpful.


On 9 August 2017 at 16:17, Diana Picus  wrote:

Hi,

See attached. FWIW, when I ran this on a very similar machine, I got
194 failures, all of which went away after reverting. So there might
be something fishy going on.

Regards,
Diana

On 9 August 2017 at 15:02, Vassil Vassilev 
wrote:

Hi Diana,

   It seems the service is down. Could you send us the details of the
failures (incl stack traces if any)

Many thanks,
Vassil


On 09/08/17 15:27, Diana Picus via cfe-commits wrote:

Hi Richard,

I'm sorry but I've reverted this in r310464 because it was breaking
some ASAN tests on this bot:


http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/9452

Please let me know if I can help debug this.

Cheers,
Diana

On 8 August 2017 at 21:14, Richard Smith via cfe-commits
 wrote:

I forgot to say:

Based on a patch by Vassil Vassilev, which was based on a patch by
Bernd
Schmidt, which was based on a patch by Reid Kleckner.

On 8 August 2017 at 12:12, Richard Smith via cfe-commits
 wrote:

Author: rsmith
Date: Tue Aug  8 12:12:28 2017

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Vassil Vassilev via cfe-commits]

Hi Diana,

On 14/08/17 11:27, Diana Picus wrote:

Hi,

Strangely enough, it turns out that if I run
Asan-armhf-with-calls-Noinst-Test on the command line it fails,
although it doesn't fail when run with lit. I've attached the stack
trace from gdb. It looks like some trouble passing down va_arg
parameters, but I haven't looked into too much details. The segfault
happens when we try to do a   ldrb   r3, [r0, r1], with r1 set to 0 by
the current function and r0 passed down from the caller. I'm not sure
if this is the exact same problem as the other tests, but feel free to
have a look at that code.

  That smells like our patch is interfering in some way...


Meanwhile, I've removed some clutter from Asan-armhf-with-calls-Test
(which is the original failure that we were seeing) and left only one
failing test that seemed small enough. I'll try to look at the
disassembly before/after the patch and maybe even run valgrind on it
(running it on the original binary naturally takes forever).

Let me know if there's anything else I could try. I can also send you
disassembly or even LLVM IR for the Asan-armhf-with-calls-Noinst-Test
if you think it helps.
  Would you be able to run clang -E on the translation unit that 
crashes and send it over? If you could give us a minimal reproducer it 
would be even better.


Cheers, Vassil


Cheers,
Diana

On 11 August 2017 at 15:34, Diana Picus  wrote:

Well, these are ASAN tests, I'm not sure how that would interact with Valgrind.
Anyway, I'll try to reproduce the environment, I'm guessing it would
be best to catch this in gdb so I can actually see what's going on.

On 11 August 2017 at 15:21, Vassil Vassilev  wrote:

That's really strange. It looks like some random behavior. Did you run some 
memory checker like valgrind?

Do the environment provided by the test runner and yours match?

Sent from my phone. Please excuse my brevity.


On 11 Aug 2017, at 15:58, Diana Picus  wrote:

Hi again,

I finally got the debug build, but unfortunately the stack traces that
the tests print look the same. My suspicion is that this is because
the addresses printed by the tests are funny (i.e. odd numbers instead
of divisible by 4). I tried to follow those addresses in an objdump of
the executable, but I didn't have much success since most of them
weren't really pointing to call instructions.

When I try to run the tests manually in the shell or in gdb, they pass.

I'm not sure what else to try. Thoughts?

Thanks,
Diana


On 11 August 2017 at 11:14, Diana Picus  wrote:
Hi guys,

I'm SO sorry about the delays. I've been having all sorts of trouble
getting that debug build on the board (from ld running out of memory
to the board just crashing on me, in which case I need to ask someone
else to reboot it because I can't power cycle it remotely). I can
assure you this is one of my top priorities, I'll get those stack
traces as soon as I can.

Thanks for your patience and sorry again,
Diana


On 10 August 2017 at 22:55, Richard Smith  wrote:
Any news on this? We want this change in Clang 5, so the sooner we can
understand and fix this regression the better...

On 10 August 2017 at 01:28, Diana Picus via cfe-commits
 wrote:

Hi Vassil,

My build is in progress, but since it's a full build it's probably
going to take another couple of hours to complete. I'll let you know
when it's done.

Thanks,
Diana

On 10 August 2017 at 10:09, Vassil Vassilev 
wrote:

It looks like I can not reproduce it on osx (non-arm)... :(

On 09/08/17 22:54, Diana Picus wrote:

Reverting this also fixed the selfhost bots:


http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15-full-sh/builds/2142


http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost/builds/2309


http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost-neon/builds/1819

I'm afraid the logs for those look even less helpful.


On 9 August 2017 at 16:17, Diana Picus  wrote:

Hi,

See attached. FWIW, when I ran this on a very similar machine, I got
194 failures, all of which went away after reverting. So there might
be something fishy going on.

Regards,
Diana

On 9 August 2017 at 15:02, Vassil Vassilev 
wrote:

Hi Diana,

   It seems the service is down. Could you send us the details of the
failures (incl stack traces if any)

Many thanks,
Vassil


On 09/08/17 15:27, Diana Picus via cfe-commits wrote:

Hi Richard,

I'm sorry but I've reverted this in r310464 because it was breaking
some ASAN tests on this bot:


http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/9452

Please let me know if I can help debug this.

Cheers,
Diana

On 8 August 2017 at 21:14, Richard Smith via cfe-commits
 wrote:

I forgot to say:

Based on a patch by Vassil Vassilev, which was based on a patch by

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Diana Picus via cfe-commits]

Hi,

Strangely enough, it turns out that if I run
Asan-armhf-with-calls-Noinst-Test on the command line it fails,
although it doesn't fail when run with lit. I've attached the stack
trace from gdb. It looks like some trouble passing down va_arg
parameters, but I haven't looked into too much details. The segfault
happens when we try to do a   ldrb   r3, [r0, r1], with r1 set to 0 by
the current function and r0 passed down from the caller. I'm not sure
if this is the exact same problem as the other tests, but feel free to
have a look at that code.

Meanwhile, I've removed some clutter from Asan-armhf-with-calls-Test
(which is the original failure that we were seeing) and left only one
failing test that seemed small enough. I'll try to look at the
disassembly before/after the patch and maybe even run valgrind on it
(running it on the original binary naturally takes forever).

Let me know if there's anything else I could try. I can also send you
disassembly or even LLVM IR for the Asan-armhf-with-calls-Noinst-Test
if you think it helps.

Cheers,
Diana

On 11 August 2017 at 15:34, Diana Picus  wrote:
> Well, these are ASAN tests, I'm not sure how that would interact with 
> Valgrind.
> Anyway, I'll try to reproduce the environment, I'm guessing it would
> be best to catch this in gdb so I can actually see what's going on.
>
> On 11 August 2017 at 15:21, Vassil Vassilev  wrote:
>> That's really strange. It looks like some random behavior. Did you run some 
>> memory checker like valgrind?
>>
>> Do the environment provided by the test runner and yours match?
>>
>> Sent from my phone. Please excuse my brevity.
>>
>>> On 11 Aug 2017, at 15:58, Diana Picus  wrote:
>>>
>>> Hi again,
>>>
>>> I finally got the debug build, but unfortunately the stack traces that
>>> the tests print look the same. My suspicion is that this is because
>>> the addresses printed by the tests are funny (i.e. odd numbers instead
>>> of divisible by 4). I tried to follow those addresses in an objdump of
>>> the executable, but I didn't have much success since most of them
>>> weren't really pointing to call instructions.
>>>
>>> When I try to run the tests manually in the shell or in gdb, they pass.
>>>
>>> I'm not sure what else to try. Thoughts?
>>>
>>> Thanks,
>>> Diana
>>>
 On 11 August 2017 at 11:14, Diana Picus  wrote:
 Hi guys,

 I'm SO sorry about the delays. I've been having all sorts of trouble
 getting that debug build on the board (from ld running out of memory
 to the board just crashing on me, in which case I need to ask someone
 else to reboot it because I can't power cycle it remotely). I can
 assure you this is one of my top priorities, I'll get those stack
 traces as soon as I can.

 Thanks for your patience and sorry again,
 Diana

> On 10 August 2017 at 22:55, Richard Smith  wrote:
> Any news on this? We want this change in Clang 5, so the sooner we can
> understand and fix this regression the better...
>
> On 10 August 2017 at 01:28, Diana Picus via cfe-commits
>  wrote:
>>
>> Hi Vassil,
>>
>> My build is in progress, but since it's a full build it's probably
>> going to take another couple of hours to complete. I'll let you know
>> when it's done.
>>
>> Thanks,
>> Diana
>>
>> On 10 August 2017 at 10:09, Vassil Vassilev 
>> wrote:
>>> It looks like I can not reproduce it on osx (non-arm)... :(
 On 09/08/17 22:54, Diana Picus wrote:

 Reverting this also fixed the selfhost bots:


 http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15-full-sh/builds/2142


 http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost/builds/2309


 http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost-neon/builds/1819

 I'm afraid the logs for those look even less helpful.

> On 9 August 2017 at 16:17, Diana Picus  wrote:
>
> Hi,
>
> See attached. FWIW, when I ran this on a very similar machine, I got
> 194 failures, all of which went away after reverting. So there might
> be something fishy going on.
>
> Regards,
> Diana
>
> On 9 August 2017 at 15:02, Vassil Vassilev 
> wrote:
>>
>> Hi Diana,
>>
>>   It seems the service is down. Could you send us the details of the
>> failures (incl stack traces if any)
>>
>> Many thanks,
>> Vassil
>>
>>> On 09/08/17 15:27, Diana Picus via cfe-commits wrote:
>>>
>>> Hi Richard,
>>>
>>> I'm sorry but I've reverted 

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Diana Picus via cfe-commits]

Well, these are ASAN tests, I'm not sure how that would interact with Valgrind.
Anyway, I'll try to reproduce the environment, I'm guessing it would
be best to catch this in gdb so I can actually see what's going on.

On 11 August 2017 at 15:21, Vassil Vassilev  wrote:
> That's really strange. It looks like some random behavior. Did you run some 
> memory checker like valgrind?
>
> Do the environment provided by the test runner and yours match?
>
> Sent from my phone. Please excuse my brevity.
>
>> On 11 Aug 2017, at 15:58, Diana Picus  wrote:
>>
>> Hi again,
>>
>> I finally got the debug build, but unfortunately the stack traces that
>> the tests print look the same. My suspicion is that this is because
>> the addresses printed by the tests are funny (i.e. odd numbers instead
>> of divisible by 4). I tried to follow those addresses in an objdump of
>> the executable, but I didn't have much success since most of them
>> weren't really pointing to call instructions.
>>
>> When I try to run the tests manually in the shell or in gdb, they pass.
>>
>> I'm not sure what else to try. Thoughts?
>>
>> Thanks,
>> Diana
>>
>>> On 11 August 2017 at 11:14, Diana Picus  wrote:
>>> Hi guys,
>>>
>>> I'm SO sorry about the delays. I've been having all sorts of trouble
>>> getting that debug build on the board (from ld running out of memory
>>> to the board just crashing on me, in which case I need to ask someone
>>> else to reboot it because I can't power cycle it remotely). I can
>>> assure you this is one of my top priorities, I'll get those stack
>>> traces as soon as I can.
>>>
>>> Thanks for your patience and sorry again,
>>> Diana
>>>
 On 10 August 2017 at 22:55, Richard Smith  wrote:
 Any news on this? We want this change in Clang 5, so the sooner we can
 understand and fix this regression the better...

 On 10 August 2017 at 01:28, Diana Picus via cfe-commits
  wrote:
>
> Hi Vassil,
>
> My build is in progress, but since it's a full build it's probably
> going to take another couple of hours to complete. I'll let you know
> when it's done.
>
> Thanks,
> Diana
>
> On 10 August 2017 at 10:09, Vassil Vassilev 
> wrote:
>> It looks like I can not reproduce it on osx (non-arm)... :(
>>> On 09/08/17 22:54, Diana Picus wrote:
>>>
>>> Reverting this also fixed the selfhost bots:
>>>
>>>
>>> http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15-full-sh/builds/2142
>>>
>>>
>>> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost/builds/2309
>>>
>>>
>>> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost-neon/builds/1819
>>>
>>> I'm afraid the logs for those look even less helpful.
>>>
 On 9 August 2017 at 16:17, Diana Picus  wrote:

 Hi,

 See attached. FWIW, when I ran this on a very similar machine, I got
 194 failures, all of which went away after reverting. So there might
 be something fishy going on.

 Regards,
 Diana

 On 9 August 2017 at 15:02, Vassil Vassilev 
 wrote:
>
> Hi Diana,
>
>   It seems the service is down. Could you send us the details of the
> failures (incl stack traces if any)
>
> Many thanks,
> Vassil
>
>> On 09/08/17 15:27, Diana Picus via cfe-commits wrote:
>>
>> Hi Richard,
>>
>> I'm sorry but I've reverted this in r310464 because it was breaking
>> some ASAN tests on this bot:
>>
>>
>> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/9452
>>
>> Please let me know if I can help debug this.
>>
>> Cheers,
>> Diana
>>
>> On 8 August 2017 at 21:14, Richard Smith via cfe-commits
>>  wrote:
>>>
>>> I forgot to say:
>>>
>>> Based on a patch by Vassil Vassilev, which was based on a patch by
>>> Bernd
>>> Schmidt, which was based on a patch by Reid Kleckner.
>>>
>>> On 8 August 2017 at 12:12, Richard Smith via cfe-commits
>>>  wrote:

 Author: rsmith
 Date: Tue Aug  8 12:12:28 2017
 New Revision: 310401

 URL: http://llvm.org/viewvc/llvm-project?rev=310401=rev
 Log:
 PR19668, PR23034: Fix handling of move constructors and deleted
 copy
 constructors when deciding whether classes should be passed
 indirectly.

 This fixes ABI differences between Clang 

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Vassil Vassilev via cfe-commits]

That's really strange. It looks like some random behavior. Did you run some 
memory checker like valgrind?

Do the environment provided by the test runner and yours match?

Sent from my phone. Please excuse my brevity.

> On 11 Aug 2017, at 15:58, Diana Picus  wrote:
> 
> Hi again,
> 
> I finally got the debug build, but unfortunately the stack traces that
> the tests print look the same. My suspicion is that this is because
> the addresses printed by the tests are funny (i.e. odd numbers instead
> of divisible by 4). I tried to follow those addresses in an objdump of
> the executable, but I didn't have much success since most of them
> weren't really pointing to call instructions.
> 
> When I try to run the tests manually in the shell or in gdb, they pass.
> 
> I'm not sure what else to try. Thoughts?
> 
> Thanks,
> Diana
> 
>> On 11 August 2017 at 11:14, Diana Picus  wrote:
>> Hi guys,
>> 
>> I'm SO sorry about the delays. I've been having all sorts of trouble
>> getting that debug build on the board (from ld running out of memory
>> to the board just crashing on me, in which case I need to ask someone
>> else to reboot it because I can't power cycle it remotely). I can
>> assure you this is one of my top priorities, I'll get those stack
>> traces as soon as I can.
>> 
>> Thanks for your patience and sorry again,
>> Diana
>> 
>>> On 10 August 2017 at 22:55, Richard Smith  wrote:
>>> Any news on this? We want this change in Clang 5, so the sooner we can
>>> understand and fix this regression the better...
>>> 
>>> On 10 August 2017 at 01:28, Diana Picus via cfe-commits
>>>  wrote:
 
 Hi Vassil,
 
 My build is in progress, but since it's a full build it's probably
 going to take another couple of hours to complete. I'll let you know
 when it's done.
 
 Thanks,
 Diana
 
 On 10 August 2017 at 10:09, Vassil Vassilev 
 wrote:
> It looks like I can not reproduce it on osx (non-arm)... :(
>> On 09/08/17 22:54, Diana Picus wrote:
>> 
>> Reverting this also fixed the selfhost bots:
>> 
>> 
>> http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15-full-sh/builds/2142
>> 
>> 
>> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost/builds/2309
>> 
>> 
>> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost-neon/builds/1819
>> 
>> I'm afraid the logs for those look even less helpful.
>> 
>>> On 9 August 2017 at 16:17, Diana Picus  wrote:
>>> 
>>> Hi,
>>> 
>>> See attached. FWIW, when I ran this on a very similar machine, I got
>>> 194 failures, all of which went away after reverting. So there might
>>> be something fishy going on.
>>> 
>>> Regards,
>>> Diana
>>> 
>>> On 9 August 2017 at 15:02, Vassil Vassilev 
>>> wrote:
 
 Hi Diana,
 
   It seems the service is down. Could you send us the details of the
 failures (incl stack traces if any)
 
 Many thanks,
 Vassil
 
> On 09/08/17 15:27, Diana Picus via cfe-commits wrote:
> 
> Hi Richard,
> 
> I'm sorry but I've reverted this in r310464 because it was breaking
> some ASAN tests on this bot:
> 
> 
> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/9452
> 
> Please let me know if I can help debug this.
> 
> Cheers,
> Diana
> 
> On 8 August 2017 at 21:14, Richard Smith via cfe-commits
>  wrote:
>> 
>> I forgot to say:
>> 
>> Based on a patch by Vassil Vassilev, which was based on a patch by
>> Bernd
>> Schmidt, which was based on a patch by Reid Kleckner.
>> 
>> On 8 August 2017 at 12:12, Richard Smith via cfe-commits
>>  wrote:
>>> 
>>> Author: rsmith
>>> Date: Tue Aug  8 12:12:28 2017
>>> New Revision: 310401
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=310401=rev
>>> Log:
>>> PR19668, PR23034: Fix handling of move constructors and deleted
>>> copy
>>> constructors when deciding whether classes should be passed
>>> indirectly.
>>> 
>>> This fixes ABI differences between Clang and GCC:
>>> 
>>>   * Previously, Clang ignored the move constructor when making
>>> this
>>> determination. It now takes the move constructor into
>>> account,
>>> per
>>> https://github.com/itanium-cxx-abi/cxx-abi/pull/17 (this
>>> change
>>> may
>>> seem recent, but the ABI change was 

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Diana Picus via cfe-commits]

Hi again,

I finally got the debug build, but unfortunately the stack traces that
the tests print look the same. My suspicion is that this is because
the addresses printed by the tests are funny (i.e. odd numbers instead
of divisible by 4). I tried to follow those addresses in an objdump of
the executable, but I didn't have much success since most of them
weren't really pointing to call instructions.

When I try to run the tests manually in the shell or in gdb, they pass.

I'm not sure what else to try. Thoughts?

Thanks,
Diana

On 11 August 2017 at 11:14, Diana Picus  wrote:
> Hi guys,
>
> I'm SO sorry about the delays. I've been having all sorts of trouble
> getting that debug build on the board (from ld running out of memory
> to the board just crashing on me, in which case I need to ask someone
> else to reboot it because I can't power cycle it remotely). I can
> assure you this is one of my top priorities, I'll get those stack
> traces as soon as I can.
>
> Thanks for your patience and sorry again,
> Diana
>
> On 10 August 2017 at 22:55, Richard Smith  wrote:
>> Any news on this? We want this change in Clang 5, so the sooner we can
>> understand and fix this regression the better...
>>
>> On 10 August 2017 at 01:28, Diana Picus via cfe-commits
>>  wrote:
>>>
>>> Hi Vassil,
>>>
>>> My build is in progress, but since it's a full build it's probably
>>> going to take another couple of hours to complete. I'll let you know
>>> when it's done.
>>>
>>> Thanks,
>>> Diana
>>>
>>> On 10 August 2017 at 10:09, Vassil Vassilev 
>>> wrote:
>>> > It looks like I can not reproduce it on osx (non-arm)... :(
>>> > On 09/08/17 22:54, Diana Picus wrote:
>>> >>
>>> >> Reverting this also fixed the selfhost bots:
>>> >>
>>> >>
>>> >> http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15-full-sh/builds/2142
>>> >>
>>> >>
>>> >> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost/builds/2309
>>> >>
>>> >>
>>> >> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost-neon/builds/1819
>>> >>
>>> >> I'm afraid the logs for those look even less helpful.
>>> >>
>>> >> On 9 August 2017 at 16:17, Diana Picus  wrote:
>>> >>>
>>> >>> Hi,
>>> >>>
>>> >>> See attached. FWIW, when I ran this on a very similar machine, I got
>>> >>> 194 failures, all of which went away after reverting. So there might
>>> >>> be something fishy going on.
>>> >>>
>>> >>> Regards,
>>> >>> Diana
>>> >>>
>>> >>> On 9 August 2017 at 15:02, Vassil Vassilev 
>>> >>> wrote:
>>> 
>>>  Hi Diana,
>>> 
>>> It seems the service is down. Could you send us the details of the
>>>  failures (incl stack traces if any)
>>> 
>>>  Many thanks,
>>>  Vassil
>>> 
>>>  On 09/08/17 15:27, Diana Picus via cfe-commits wrote:
>>> >
>>> > Hi Richard,
>>> >
>>> > I'm sorry but I've reverted this in r310464 because it was breaking
>>> > some ASAN tests on this bot:
>>> >
>>> >
>>> > http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/9452
>>> >
>>> > Please let me know if I can help debug this.
>>> >
>>> > Cheers,
>>> > Diana
>>> >
>>> > On 8 August 2017 at 21:14, Richard Smith via cfe-commits
>>> >  wrote:
>>> >>
>>> >> I forgot to say:
>>> >>
>>> >> Based on a patch by Vassil Vassilev, which was based on a patch by
>>> >> Bernd
>>> >> Schmidt, which was based on a patch by Reid Kleckner.
>>> >>
>>> >> On 8 August 2017 at 12:12, Richard Smith via cfe-commits
>>> >>  wrote:
>>> >>>
>>> >>> Author: rsmith
>>> >>> Date: Tue Aug  8 12:12:28 2017
>>> >>> New Revision: 310401
>>> >>>
>>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=310401=rev
>>> >>> Log:
>>> >>> PR19668, PR23034: Fix handling of move constructors and deleted
>>> >>> copy
>>> >>> constructors when deciding whether classes should be passed
>>> >>> indirectly.
>>> >>>
>>> >>> This fixes ABI differences between Clang and GCC:
>>> >>>
>>> >>>* Previously, Clang ignored the move constructor when making
>>> >>> this
>>> >>>  determination. It now takes the move constructor into
>>> >>> account,
>>> >>> per
>>> >>>  https://github.com/itanium-cxx-abi/cxx-abi/pull/17 (this
>>> >>> change
>>> >>> may
>>> >>>  seem recent, but the ABI change was agreed on the Itanium C++
>>> >>> ABI
>>> >>>  list a long time ago).
>>> >>>
>>> >>>* Previously, Clang's behavior when the copy constructor was
>>> >>> deleted
>>> >>>  was unstable -- depending on whether the lazy declaration of
>>> >>> the
>>> >>>  copy constructor had been triggered, you might get different
>>> >>> behavior.
>>> >>>  We 

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Diana Picus via cfe-commits]

Hi guys,

I'm SO sorry about the delays. I've been having all sorts of trouble
getting that debug build on the board (from ld running out of memory
to the board just crashing on me, in which case I need to ask someone
else to reboot it because I can't power cycle it remotely). I can
assure you this is one of my top priorities, I'll get those stack
traces as soon as I can.

Thanks for your patience and sorry again,
Diana

On 10 August 2017 at 22:55, Richard Smith  wrote:
> Any news on this? We want this change in Clang 5, so the sooner we can
> understand and fix this regression the better...
>
> On 10 August 2017 at 01:28, Diana Picus via cfe-commits
>  wrote:
>>
>> Hi Vassil,
>>
>> My build is in progress, but since it's a full build it's probably
>> going to take another couple of hours to complete. I'll let you know
>> when it's done.
>>
>> Thanks,
>> Diana
>>
>> On 10 August 2017 at 10:09, Vassil Vassilev 
>> wrote:
>> > It looks like I can not reproduce it on osx (non-arm)... :(
>> > On 09/08/17 22:54, Diana Picus wrote:
>> >>
>> >> Reverting this also fixed the selfhost bots:
>> >>
>> >>
>> >> http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15-full-sh/builds/2142
>> >>
>> >>
>> >> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost/builds/2309
>> >>
>> >>
>> >> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost-neon/builds/1819
>> >>
>> >> I'm afraid the logs for those look even less helpful.
>> >>
>> >> On 9 August 2017 at 16:17, Diana Picus  wrote:
>> >>>
>> >>> Hi,
>> >>>
>> >>> See attached. FWIW, when I ran this on a very similar machine, I got
>> >>> 194 failures, all of which went away after reverting. So there might
>> >>> be something fishy going on.
>> >>>
>> >>> Regards,
>> >>> Diana
>> >>>
>> >>> On 9 August 2017 at 15:02, Vassil Vassilev 
>> >>> wrote:
>> 
>>  Hi Diana,
>> 
>> It seems the service is down. Could you send us the details of the
>>  failures (incl stack traces if any)
>> 
>>  Many thanks,
>>  Vassil
>> 
>>  On 09/08/17 15:27, Diana Picus via cfe-commits wrote:
>> >
>> > Hi Richard,
>> >
>> > I'm sorry but I've reverted this in r310464 because it was breaking
>> > some ASAN tests on this bot:
>> >
>> >
>> > http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/9452
>> >
>> > Please let me know if I can help debug this.
>> >
>> > Cheers,
>> > Diana
>> >
>> > On 8 August 2017 at 21:14, Richard Smith via cfe-commits
>> >  wrote:
>> >>
>> >> I forgot to say:
>> >>
>> >> Based on a patch by Vassil Vassilev, which was based on a patch by
>> >> Bernd
>> >> Schmidt, which was based on a patch by Reid Kleckner.
>> >>
>> >> On 8 August 2017 at 12:12, Richard Smith via cfe-commits
>> >>  wrote:
>> >>>
>> >>> Author: rsmith
>> >>> Date: Tue Aug  8 12:12:28 2017
>> >>> New Revision: 310401
>> >>>
>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=310401=rev
>> >>> Log:
>> >>> PR19668, PR23034: Fix handling of move constructors and deleted
>> >>> copy
>> >>> constructors when deciding whether classes should be passed
>> >>> indirectly.
>> >>>
>> >>> This fixes ABI differences between Clang and GCC:
>> >>>
>> >>>* Previously, Clang ignored the move constructor when making
>> >>> this
>> >>>  determination. It now takes the move constructor into
>> >>> account,
>> >>> per
>> >>>  https://github.com/itanium-cxx-abi/cxx-abi/pull/17 (this
>> >>> change
>> >>> may
>> >>>  seem recent, but the ABI change was agreed on the Itanium C++
>> >>> ABI
>> >>>  list a long time ago).
>> >>>
>> >>>* Previously, Clang's behavior when the copy constructor was
>> >>> deleted
>> >>>  was unstable -- depending on whether the lazy declaration of
>> >>> the
>> >>>  copy constructor had been triggered, you might get different
>> >>> behavior.
>> >>>  We now eagerly declare the copy constructor whenever its
>> >>> deletedness
>> >>>  is unclear, and ignore deleted copy/move constructors when
>> >>> looking
>> >>> for
>> >>>  a trivial such constructor.
>> >>>
>> >>> This also fixes an ABI difference between Clang and MSVC:
>> >>>
>> >>>* If the copy constructor would be implicitly deleted (but has
>> >>> not
>> >>> been
>> >>>  lazily declared yet), for instance because the class has an
>> >>> rvalue
>> >>>  reference member, we would pass it directly. We now pass such
>> >>> a
>> >>> class
>> >>>  indirectly, matching MSVC.
>> >>>
>> >>> Modified:
>> >>>   

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Vassil Vassilev via cfe-commits]

Hi Diana,

  Thanks for helping us out!

Cheers, Vassil
On 10/08/17 11:28, Diana Picus wrote:

Hi Vassil,

My build is in progress, but since it's a full build it's probably
going to take another couple of hours to complete. I'll let you know
when it's done.

Thanks,
Diana

On 10 August 2017 at 10:09, Vassil Vassilev  wrote:

It looks like I can not reproduce it on osx (non-arm)... :(
On 09/08/17 22:54, Diana Picus wrote:

Reverting this also fixed the selfhost bots:

http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15-full-sh/builds/2142

http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost/builds/2309

http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost-neon/builds/1819

I'm afraid the logs for those look even less helpful.

On 9 August 2017 at 16:17, Diana Picus  wrote:

Hi,

See attached. FWIW, when I ran this on a very similar machine, I got
194 failures, all of which went away after reverting. So there might
be something fishy going on.

Regards,
Diana

On 9 August 2017 at 15:02, Vassil Vassilev 
wrote:

Hi Diana,

It seems the service is down. Could you send us the details of the
failures (incl stack traces if any)

Many thanks,
Vassil

On 09/08/17 15:27, Diana Picus via cfe-commits wrote:

Hi Richard,

I'm sorry but I've reverted this in r310464 because it was breaking
some ASAN tests on this bot:

http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/9452

Please let me know if I can help debug this.

Cheers,
Diana

On 8 August 2017 at 21:14, Richard Smith via cfe-commits
 wrote:

I forgot to say:

Based on a patch by Vassil Vassilev, which was based on a patch by
Bernd
Schmidt, which was based on a patch by Reid Kleckner.

On 8 August 2017 at 12:12, Richard Smith via cfe-commits
 wrote:

Author: rsmith
Date: Tue Aug  8 12:12:28 2017
New Revision: 310401

URL: http://llvm.org/viewvc/llvm-project?rev=310401=rev
Log:
PR19668, PR23034: Fix handling of move constructors and deleted copy
constructors when deciding whether classes should be passed
indirectly.

This fixes ABI differences between Clang and GCC:

* Previously, Clang ignored the move constructor when making this
  determination. It now takes the move constructor into account,
per
  https://github.com/itanium-cxx-abi/cxx-abi/pull/17 (this change
may
  seem recent, but the ABI change was agreed on the Itanium C++
ABI
  list a long time ago).

* Previously, Clang's behavior when the copy constructor was
deleted
  was unstable -- depending on whether the lazy declaration of the
  copy constructor had been triggered, you might get different
behavior.
  We now eagerly declare the copy constructor whenever its
deletedness
  is unclear, and ignore deleted copy/move constructors when
looking
for
  a trivial such constructor.

This also fixes an ABI difference between Clang and MSVC:

* If the copy constructor would be implicitly deleted (but has not
been
  lazily declared yet), for instance because the class has an
rvalue
  reference member, we would pass it directly. We now pass such a
class
  indirectly, matching MSVC.

Modified:
   cfe/trunk/include/clang/AST/DeclCXX.h
   cfe/trunk/lib/AST/ASTImporter.cpp
   cfe/trunk/lib/AST/DeclCXX.cpp
   cfe/trunk/lib/CodeGen/CGCXXABI.cpp
   cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
   cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
   cfe/trunk/lib/Sema/SemaDeclCXX.cpp
   cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
   cfe/trunk/lib/Serialization/ASTWriter.cpp
   cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp
   cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL:


http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=310401=310400=310401=diff



==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Tue Aug  8 12:12:28 2017
@@ -375,6 +375,7 @@ class CXXRecordDecl : public RecordDecl
/// \brief These flags are \c true if a defaulted
corresponding
special
/// member can't be fully analyzed without performing overload
resolution.
/// @{
+unsigned NeedOverloadResolutionForCopyConstructor : 1;
unsigned NeedOverloadResolutionForMoveConstructor : 1;
unsigned NeedOverloadResolutionForMoveAssignment : 1;
unsigned NeedOverloadResolutionForDestructor : 1;
@@ -383,6 +384,7 @@ class CXXRecordDecl : public RecordDecl
/// \brief These flags are \c true if an implicit defaulted
corresponding
/// special member would be defined as deleted.
/// @{
+unsigned DefaultedCopyConstructorIsDeleted : 1;
unsigned DefaultedMoveConstructorIsDeleted : 1;
unsigned 

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Diana Picus via cfe-commits]

Hi Vassil,

My build is in progress, but since it's a full build it's probably
going to take another couple of hours to complete. I'll let you know
when it's done.

Thanks,
Diana

On 10 August 2017 at 10:09, Vassil Vassilev  wrote:
> It looks like I can not reproduce it on osx (non-arm)... :(
> On 09/08/17 22:54, Diana Picus wrote:
>>
>> Reverting this also fixed the selfhost bots:
>>
>> http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15-full-sh/builds/2142
>>
>> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost/builds/2309
>>
>> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost-neon/builds/1819
>>
>> I'm afraid the logs for those look even less helpful.
>>
>> On 9 August 2017 at 16:17, Diana Picus  wrote:
>>>
>>> Hi,
>>>
>>> See attached. FWIW, when I ran this on a very similar machine, I got
>>> 194 failures, all of which went away after reverting. So there might
>>> be something fishy going on.
>>>
>>> Regards,
>>> Diana
>>>
>>> On 9 August 2017 at 15:02, Vassil Vassilev 
>>> wrote:

 Hi Diana,

It seems the service is down. Could you send us the details of the
 failures (incl stack traces if any)

 Many thanks,
 Vassil

 On 09/08/17 15:27, Diana Picus via cfe-commits wrote:
>
> Hi Richard,
>
> I'm sorry but I've reverted this in r310464 because it was breaking
> some ASAN tests on this bot:
>
> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/9452
>
> Please let me know if I can help debug this.
>
> Cheers,
> Diana
>
> On 8 August 2017 at 21:14, Richard Smith via cfe-commits
>  wrote:
>>
>> I forgot to say:
>>
>> Based on a patch by Vassil Vassilev, which was based on a patch by
>> Bernd
>> Schmidt, which was based on a patch by Reid Kleckner.
>>
>> On 8 August 2017 at 12:12, Richard Smith via cfe-commits
>>  wrote:
>>>
>>> Author: rsmith
>>> Date: Tue Aug  8 12:12:28 2017
>>> New Revision: 310401
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=310401=rev
>>> Log:
>>> PR19668, PR23034: Fix handling of move constructors and deleted copy
>>> constructors when deciding whether classes should be passed
>>> indirectly.
>>>
>>> This fixes ABI differences between Clang and GCC:
>>>
>>>* Previously, Clang ignored the move constructor when making this
>>>  determination. It now takes the move constructor into account,
>>> per
>>>  https://github.com/itanium-cxx-abi/cxx-abi/pull/17 (this change
>>> may
>>>  seem recent, but the ABI change was agreed on the Itanium C++
>>> ABI
>>>  list a long time ago).
>>>
>>>* Previously, Clang's behavior when the copy constructor was
>>> deleted
>>>  was unstable -- depending on whether the lazy declaration of the
>>>  copy constructor had been triggered, you might get different
>>> behavior.
>>>  We now eagerly declare the copy constructor whenever its
>>> deletedness
>>>  is unclear, and ignore deleted copy/move constructors when
>>> looking
>>> for
>>>  a trivial such constructor.
>>>
>>> This also fixes an ABI difference between Clang and MSVC:
>>>
>>>* If the copy constructor would be implicitly deleted (but has not
>>> been
>>>  lazily declared yet), for instance because the class has an
>>> rvalue
>>>  reference member, we would pass it directly. We now pass such a
>>> class
>>>  indirectly, matching MSVC.
>>>
>>> Modified:
>>>   cfe/trunk/include/clang/AST/DeclCXX.h
>>>   cfe/trunk/lib/AST/ASTImporter.cpp
>>>   cfe/trunk/lib/AST/DeclCXX.cpp
>>>   cfe/trunk/lib/CodeGen/CGCXXABI.cpp
>>>   cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
>>>   cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
>>>   cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>>>   cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>>>   cfe/trunk/lib/Serialization/ASTWriter.cpp
>>>   cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp
>>>   cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
>>>
>>> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
>>> URL:
>>>
>>>
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=310401=310400=310401=diff
>>>
>>>
>>>
>>> ==
>>> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
>>> +++ cfe/trunk/include/clang/AST/DeclCXX.h Tue Aug  8 12:12:28 2017
>>> @@ -375,6 +375,7 @@ class CXXRecordDecl : public RecordDecl
>>>/// \brief These flags are \c true if a defaulted
>>> corresponding

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Vassil Vassilev via cfe-commits]

It looks like I can not reproduce it on osx (non-arm)... :(
On 09/08/17 22:54, Diana Picus wrote:

Reverting this also fixed the selfhost bots:
http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15-full-sh/builds/2142
http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost/builds/2309
http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost-neon/builds/1819

I'm afraid the logs for those look even less helpful.

On 9 August 2017 at 16:17, Diana Picus  wrote:

Hi,

See attached. FWIW, when I ran this on a very similar machine, I got
194 failures, all of which went away after reverting. So there might
be something fishy going on.

Regards,
Diana

On 9 August 2017 at 15:02, Vassil Vassilev  wrote:

Hi Diana,

   It seems the service is down. Could you send us the details of the
failures (incl stack traces if any)

Many thanks,
Vassil

On 09/08/17 15:27, Diana Picus via cfe-commits wrote:

Hi Richard,

I'm sorry but I've reverted this in r310464 because it was breaking
some ASAN tests on this bot:
http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/9452

Please let me know if I can help debug this.

Cheers,
Diana

On 8 August 2017 at 21:14, Richard Smith via cfe-commits
 wrote:

I forgot to say:

Based on a patch by Vassil Vassilev, which was based on a patch by Bernd
Schmidt, which was based on a patch by Reid Kleckner.

On 8 August 2017 at 12:12, Richard Smith via cfe-commits
 wrote:

Author: rsmith
Date: Tue Aug  8 12:12:28 2017
New Revision: 310401

URL: http://llvm.org/viewvc/llvm-project?rev=310401=rev
Log:
PR19668, PR23034: Fix handling of move constructors and deleted copy
constructors when deciding whether classes should be passed indirectly.

This fixes ABI differences between Clang and GCC:

   * Previously, Clang ignored the move constructor when making this
 determination. It now takes the move constructor into account, per
 https://github.com/itanium-cxx-abi/cxx-abi/pull/17 (this change may
 seem recent, but the ABI change was agreed on the Itanium C++ ABI
 list a long time ago).

   * Previously, Clang's behavior when the copy constructor was deleted
 was unstable -- depending on whether the lazy declaration of the
 copy constructor had been triggered, you might get different
behavior.
 We now eagerly declare the copy constructor whenever its deletedness
 is unclear, and ignore deleted copy/move constructors when looking
for
 a trivial such constructor.

This also fixes an ABI difference between Clang and MSVC:

   * If the copy constructor would be implicitly deleted (but has not
been
 lazily declared yet), for instance because the class has an rvalue
 reference member, we would pass it directly. We now pass such a
class
 indirectly, matching MSVC.

Modified:
  cfe/trunk/include/clang/AST/DeclCXX.h
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/lib/AST/DeclCXX.cpp
  cfe/trunk/lib/CodeGen/CGCXXABI.cpp
  cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
  cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
  cfe/trunk/lib/Sema/SemaDeclCXX.cpp
  cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
  cfe/trunk/lib/Serialization/ASTWriter.cpp
  cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp
  cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL:

http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=310401=310400=310401=diff


==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Tue Aug  8 12:12:28 2017
@@ -375,6 +375,7 @@ class CXXRecordDecl : public RecordDecl
   /// \brief These flags are \c true if a defaulted corresponding
special
   /// member can't be fully analyzed without performing overload
resolution.
   /// @{
+unsigned NeedOverloadResolutionForCopyConstructor : 1;
   unsigned NeedOverloadResolutionForMoveConstructor : 1;
   unsigned NeedOverloadResolutionForMoveAssignment : 1;
   unsigned NeedOverloadResolutionForDestructor : 1;
@@ -383,6 +384,7 @@ class CXXRecordDecl : public RecordDecl
   /// \brief These flags are \c true if an implicit defaulted
corresponding
   /// special member would be defined as deleted.
   /// @{
+unsigned DefaultedCopyConstructorIsDeleted : 1;
   unsigned DefaultedMoveConstructorIsDeleted : 1;
   unsigned DefaultedMoveAssignmentIsDeleted : 1;
   unsigned DefaultedDestructorIsDeleted : 1;
@@ -415,6 +417,12 @@ class CXXRecordDecl : public RecordDecl
   /// constructor.
   unsigned HasDefaultedDefaultConstructor : 1;

+/// \brief True if this class can be passed in a
non-address-preserving
+/// fashion (such as in registers) according to the C++ language
rules.
+/// This does not 

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Vassil Vassilev via cfe-commits]

On 09/08/17 22:54, Diana Picus wrote:

Reverting this also fixed the selfhost bots:
http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15-full-sh/builds/2142
http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost/builds/2309
http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost-neon/builds/1819

I'm afraid the logs for those look even less helpful.
Not helpful indeed :) Do you have access to the machine and would you be 
willing to build in debug mode and send us some stack traces. I do not 
have any arm machines :(


On 9 August 2017 at 16:17, Diana Picus  wrote:

Hi,

See attached. FWIW, when I ran this on a very similar machine, I got
194 failures, all of which went away after reverting. So there might
be something fishy going on.

Regards,
Diana

On 9 August 2017 at 15:02, Vassil Vassilev  wrote:

Hi Diana,

   It seems the service is down. Could you send us the details of the
failures (incl stack traces if any)

Many thanks,
Vassil

On 09/08/17 15:27, Diana Picus via cfe-commits wrote:

Hi Richard,

I'm sorry but I've reverted this in r310464 because it was breaking
some ASAN tests on this bot:
http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/9452

Please let me know if I can help debug this.

Cheers,
Diana

On 8 August 2017 at 21:14, Richard Smith via cfe-commits
 wrote:

I forgot to say:

Based on a patch by Vassil Vassilev, which was based on a patch by Bernd
Schmidt, which was based on a patch by Reid Kleckner.

On 8 August 2017 at 12:12, Richard Smith via cfe-commits
 wrote:

Author: rsmith
Date: Tue Aug  8 12:12:28 2017
New Revision: 310401

URL: http://llvm.org/viewvc/llvm-project?rev=310401=rev
Log:
PR19668, PR23034: Fix handling of move constructors and deleted copy
constructors when deciding whether classes should be passed indirectly.

This fixes ABI differences between Clang and GCC:

   * Previously, Clang ignored the move constructor when making this
 determination. It now takes the move constructor into account, per
 https://github.com/itanium-cxx-abi/cxx-abi/pull/17 (this change may
 seem recent, but the ABI change was agreed on the Itanium C++ ABI
 list a long time ago).

   * Previously, Clang's behavior when the copy constructor was deleted
 was unstable -- depending on whether the lazy declaration of the
 copy constructor had been triggered, you might get different
behavior.
 We now eagerly declare the copy constructor whenever its deletedness
 is unclear, and ignore deleted copy/move constructors when looking
for
 a trivial such constructor.

This also fixes an ABI difference between Clang and MSVC:

   * If the copy constructor would be implicitly deleted (but has not
been
 lazily declared yet), for instance because the class has an rvalue
 reference member, we would pass it directly. We now pass such a
class
 indirectly, matching MSVC.

Modified:
  cfe/trunk/include/clang/AST/DeclCXX.h
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/lib/AST/DeclCXX.cpp
  cfe/trunk/lib/CodeGen/CGCXXABI.cpp
  cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
  cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
  cfe/trunk/lib/Sema/SemaDeclCXX.cpp
  cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
  cfe/trunk/lib/Serialization/ASTWriter.cpp
  cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp
  cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL:

http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=310401=310400=310401=diff


==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Tue Aug  8 12:12:28 2017
@@ -375,6 +375,7 @@ class CXXRecordDecl : public RecordDecl
   /// \brief These flags are \c true if a defaulted corresponding
special
   /// member can't be fully analyzed without performing overload
resolution.
   /// @{
+unsigned NeedOverloadResolutionForCopyConstructor : 1;
   unsigned NeedOverloadResolutionForMoveConstructor : 1;
   unsigned NeedOverloadResolutionForMoveAssignment : 1;
   unsigned NeedOverloadResolutionForDestructor : 1;
@@ -383,6 +384,7 @@ class CXXRecordDecl : public RecordDecl
   /// \brief These flags are \c true if an implicit defaulted
corresponding
   /// special member would be defined as deleted.
   /// @{
+unsigned DefaultedCopyConstructorIsDeleted : 1;
   unsigned DefaultedMoveConstructorIsDeleted : 1;
   unsigned DefaultedMoveAssignmentIsDeleted : 1;
   unsigned DefaultedDestructorIsDeleted : 1;
@@ -415,6 +417,12 @@ class CXXRecordDecl : public RecordDecl
   /// constructor.
   unsigned HasDefaultedDefaultConstructor : 1;

+/// \brief True if this class can be passed in a

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Diana Picus via cfe-commits]

Reverting this also fixed the selfhost bots:
http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-a15-full-sh/builds/2142
http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost/builds/2309
http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost-neon/builds/1819

I'm afraid the logs for those look even less helpful.

On 9 August 2017 at 16:17, Diana Picus  wrote:
> Hi,
>
> See attached. FWIW, when I ran this on a very similar machine, I got
> 194 failures, all of which went away after reverting. So there might
> be something fishy going on.
>
> Regards,
> Diana
>
> On 9 August 2017 at 15:02, Vassil Vassilev  wrote:
>> Hi Diana,
>>
>>   It seems the service is down. Could you send us the details of the
>> failures (incl stack traces if any)
>>
>> Many thanks,
>> Vassil
>>
>> On 09/08/17 15:27, Diana Picus via cfe-commits wrote:
>>>
>>> Hi Richard,
>>>
>>> I'm sorry but I've reverted this in r310464 because it was breaking
>>> some ASAN tests on this bot:
>>> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/9452
>>>
>>> Please let me know if I can help debug this.
>>>
>>> Cheers,
>>> Diana
>>>
>>> On 8 August 2017 at 21:14, Richard Smith via cfe-commits
>>>  wrote:

 I forgot to say:

 Based on a patch by Vassil Vassilev, which was based on a patch by Bernd
 Schmidt, which was based on a patch by Reid Kleckner.

 On 8 August 2017 at 12:12, Richard Smith via cfe-commits
  wrote:
>
> Author: rsmith
> Date: Tue Aug  8 12:12:28 2017
> New Revision: 310401
>
> URL: http://llvm.org/viewvc/llvm-project?rev=310401=rev
> Log:
> PR19668, PR23034: Fix handling of move constructors and deleted copy
> constructors when deciding whether classes should be passed indirectly.
>
> This fixes ABI differences between Clang and GCC:
>
>   * Previously, Clang ignored the move constructor when making this
> determination. It now takes the move constructor into account, per
> https://github.com/itanium-cxx-abi/cxx-abi/pull/17 (this change may
> seem recent, but the ABI change was agreed on the Itanium C++ ABI
> list a long time ago).
>
>   * Previously, Clang's behavior when the copy constructor was deleted
> was unstable -- depending on whether the lazy declaration of the
> copy constructor had been triggered, you might get different
> behavior.
> We now eagerly declare the copy constructor whenever its deletedness
> is unclear, and ignore deleted copy/move constructors when looking
> for
> a trivial such constructor.
>
> This also fixes an ABI difference between Clang and MSVC:
>
>   * If the copy constructor would be implicitly deleted (but has not
> been
> lazily declared yet), for instance because the class has an rvalue
> reference member, we would pass it directly. We now pass such a
> class
> indirectly, matching MSVC.
>
> Modified:
>  cfe/trunk/include/clang/AST/DeclCXX.h
>  cfe/trunk/lib/AST/ASTImporter.cpp
>  cfe/trunk/lib/AST/DeclCXX.cpp
>  cfe/trunk/lib/CodeGen/CGCXXABI.cpp
>  cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
>  cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
>  cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>  cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>  cfe/trunk/lib/Serialization/ASTWriter.cpp
>  cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp
>  cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
>
> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
> URL:
>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=310401=310400=310401=diff
>
>
> ==
> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
> +++ cfe/trunk/include/clang/AST/DeclCXX.h Tue Aug  8 12:12:28 2017
> @@ -375,6 +375,7 @@ class CXXRecordDecl : public RecordDecl
>   /// \brief These flags are \c true if a defaulted corresponding
> special
>   /// member can't be fully analyzed without performing overload
> resolution.
>   /// @{
> +unsigned NeedOverloadResolutionForCopyConstructor : 1;
>   unsigned NeedOverloadResolutionForMoveConstructor : 1;
>   unsigned NeedOverloadResolutionForMoveAssignment : 1;
>   unsigned NeedOverloadResolutionForDestructor : 1;
> @@ -383,6 +384,7 @@ class CXXRecordDecl : public RecordDecl
>   /// \brief These flags are \c true if an implicit defaulted
> corresponding
>   /// special member would be defined as deleted.
>   /// @{
> +unsigned DefaultedCopyConstructorIsDeleted : 1;
>   unsigned 

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Vassil Vassilev via cfe-commits]

Hi Diana,

  It seems the service is down. Could you send us the details of the 
failures (incl stack traces if any)


Many thanks,
Vassil
On 09/08/17 15:27, Diana Picus via cfe-commits wrote:

Hi Richard,

I'm sorry but I've reverted this in r310464 because it was breaking
some ASAN tests on this bot:
http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/9452

Please let me know if I can help debug this.

Cheers,
Diana

On 8 August 2017 at 21:14, Richard Smith via cfe-commits
 wrote:

I forgot to say:

Based on a patch by Vassil Vassilev, which was based on a patch by Bernd
Schmidt, which was based on a patch by Reid Kleckner.

On 8 August 2017 at 12:12, Richard Smith via cfe-commits
 wrote:

Author: rsmith
Date: Tue Aug  8 12:12:28 2017
New Revision: 310401

URL: http://llvm.org/viewvc/llvm-project?rev=310401=rev
Log:
PR19668, PR23034: Fix handling of move constructors and deleted copy
constructors when deciding whether classes should be passed indirectly.

This fixes ABI differences between Clang and GCC:

  * Previously, Clang ignored the move constructor when making this
determination. It now takes the move constructor into account, per
https://github.com/itanium-cxx-abi/cxx-abi/pull/17 (this change may
seem recent, but the ABI change was agreed on the Itanium C++ ABI
list a long time ago).

  * Previously, Clang's behavior when the copy constructor was deleted
was unstable -- depending on whether the lazy declaration of the
copy constructor had been triggered, you might get different behavior.
We now eagerly declare the copy constructor whenever its deletedness
is unclear, and ignore deleted copy/move constructors when looking for
a trivial such constructor.

This also fixes an ABI difference between Clang and MSVC:

  * If the copy constructor would be implicitly deleted (but has not been
lazily declared yet), for instance because the class has an rvalue
reference member, we would pass it directly. We now pass such a class
indirectly, matching MSVC.

Modified:
 cfe/trunk/include/clang/AST/DeclCXX.h
 cfe/trunk/lib/AST/ASTImporter.cpp
 cfe/trunk/lib/AST/DeclCXX.cpp
 cfe/trunk/lib/CodeGen/CGCXXABI.cpp
 cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
 cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
 cfe/trunk/lib/Sema/SemaDeclCXX.cpp
 cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
 cfe/trunk/lib/Serialization/ASTWriter.cpp
 cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp
 cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=310401=310400=310401=diff

==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Tue Aug  8 12:12:28 2017
@@ -375,6 +375,7 @@ class CXXRecordDecl : public RecordDecl
  /// \brief These flags are \c true if a defaulted corresponding
special
  /// member can't be fully analyzed without performing overload
resolution.
  /// @{
+unsigned NeedOverloadResolutionForCopyConstructor : 1;
  unsigned NeedOverloadResolutionForMoveConstructor : 1;
  unsigned NeedOverloadResolutionForMoveAssignment : 1;
  unsigned NeedOverloadResolutionForDestructor : 1;
@@ -383,6 +384,7 @@ class CXXRecordDecl : public RecordDecl
  /// \brief These flags are \c true if an implicit defaulted
corresponding
  /// special member would be defined as deleted.
  /// @{
+unsigned DefaultedCopyConstructorIsDeleted : 1;
  unsigned DefaultedMoveConstructorIsDeleted : 1;
  unsigned DefaultedMoveAssignmentIsDeleted : 1;
  unsigned DefaultedDestructorIsDeleted : 1;
@@ -415,6 +417,12 @@ class CXXRecordDecl : public RecordDecl
  /// constructor.
  unsigned HasDefaultedDefaultConstructor : 1;

+/// \brief True if this class can be passed in a
non-address-preserving
+/// fashion (such as in registers) according to the C++ language
rules.
+/// This does not imply anything about how the ABI in use will
actually
+/// pass an object of this class.
+unsigned CanPassInRegisters : 1;
+
  /// \brief True if a defaulted default constructor for this class
would
  /// be constexpr.
  unsigned DefaultedDefaultConstructorIsConstexpr : 1;
@@ -811,18 +819,50 @@ public:
  return data().FirstFriend.isValid();
}

+  /// \brief \c true if a defaulted copy constructor for this class would
be
+  /// deleted.
+  bool defaultedCopyConstructorIsDeleted() const {
+assert((!needsOverloadResolutionForCopyConstructor() ||
+(data().DeclaredSpecialMembers & SMF_CopyConstructor)) &&
+   "this property has not yet been computed by Sema");
+return data().DefaultedCopyConstructorIsDeleted;
+  }
+
+  /// \brief \c true if a defaulted 

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Diana Picus via cfe-commits]

Hi Richard,

I'm sorry but I've reverted this in r310464 because it was breaking
some ASAN tests on this bot:
http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/9452

Please let me know if I can help debug this.

Cheers,
Diana

On 8 August 2017 at 21:14, Richard Smith via cfe-commits
 wrote:
> I forgot to say:
>
> Based on a patch by Vassil Vassilev, which was based on a patch by Bernd
> Schmidt, which was based on a patch by Reid Kleckner.
>
> On 8 August 2017 at 12:12, Richard Smith via cfe-commits
>  wrote:
>>
>> Author: rsmith
>> Date: Tue Aug  8 12:12:28 2017
>> New Revision: 310401
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=310401=rev
>> Log:
>> PR19668, PR23034: Fix handling of move constructors and deleted copy
>> constructors when deciding whether classes should be passed indirectly.
>>
>> This fixes ABI differences between Clang and GCC:
>>
>>  * Previously, Clang ignored the move constructor when making this
>>determination. It now takes the move constructor into account, per
>>https://github.com/itanium-cxx-abi/cxx-abi/pull/17 (this change may
>>seem recent, but the ABI change was agreed on the Itanium C++ ABI
>>list a long time ago).
>>
>>  * Previously, Clang's behavior when the copy constructor was deleted
>>was unstable -- depending on whether the lazy declaration of the
>>copy constructor had been triggered, you might get different behavior.
>>We now eagerly declare the copy constructor whenever its deletedness
>>is unclear, and ignore deleted copy/move constructors when looking for
>>a trivial such constructor.
>>
>> This also fixes an ABI difference between Clang and MSVC:
>>
>>  * If the copy constructor would be implicitly deleted (but has not been
>>lazily declared yet), for instance because the class has an rvalue
>>reference member, we would pass it directly. We now pass such a class
>>indirectly, matching MSVC.
>>
>> Modified:
>> cfe/trunk/include/clang/AST/DeclCXX.h
>> cfe/trunk/lib/AST/ASTImporter.cpp
>> cfe/trunk/lib/AST/DeclCXX.cpp
>> cfe/trunk/lib/CodeGen/CGCXXABI.cpp
>> cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
>> cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>> cfe/trunk/lib/Serialization/ASTWriter.cpp
>> cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp
>> cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=310401=310400=310401=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
>> +++ cfe/trunk/include/clang/AST/DeclCXX.h Tue Aug  8 12:12:28 2017
>> @@ -375,6 +375,7 @@ class CXXRecordDecl : public RecordDecl
>>  /// \brief These flags are \c true if a defaulted corresponding
>> special
>>  /// member can't be fully analyzed without performing overload
>> resolution.
>>  /// @{
>> +unsigned NeedOverloadResolutionForCopyConstructor : 1;
>>  unsigned NeedOverloadResolutionForMoveConstructor : 1;
>>  unsigned NeedOverloadResolutionForMoveAssignment : 1;
>>  unsigned NeedOverloadResolutionForDestructor : 1;
>> @@ -383,6 +384,7 @@ class CXXRecordDecl : public RecordDecl
>>  /// \brief These flags are \c true if an implicit defaulted
>> corresponding
>>  /// special member would be defined as deleted.
>>  /// @{
>> +unsigned DefaultedCopyConstructorIsDeleted : 1;
>>  unsigned DefaultedMoveConstructorIsDeleted : 1;
>>  unsigned DefaultedMoveAssignmentIsDeleted : 1;
>>  unsigned DefaultedDestructorIsDeleted : 1;
>> @@ -415,6 +417,12 @@ class CXXRecordDecl : public RecordDecl
>>  /// constructor.
>>  unsigned HasDefaultedDefaultConstructor : 1;
>>
>> +/// \brief True if this class can be passed in a
>> non-address-preserving
>> +/// fashion (such as in registers) according to the C++ language
>> rules.
>> +/// This does not imply anything about how the ABI in use will
>> actually
>> +/// pass an object of this class.
>> +unsigned CanPassInRegisters : 1;
>> +
>>  /// \brief True if a defaulted default constructor for this class
>> would
>>  /// be constexpr.
>>  unsigned DefaultedDefaultConstructorIsConstexpr : 1;
>> @@ -811,18 +819,50 @@ public:
>>  return data().FirstFriend.isValid();
>>}
>>
>> +  /// \brief \c true if a defaulted copy constructor for this class would
>> be
>> +  /// deleted.
>> +  bool defaultedCopyConstructorIsDeleted() const {
>> +assert((!needsOverloadResolutionForCopyConstructor() ||
>> +(data().DeclaredSpecialMembers & SMF_CopyConstructor)) &&
>> +   "this property has not yet been computed by Sema");
>> +return 

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

[Richard Smith via cfe-commits]

I forgot to say:

Based on a patch by Vassil Vassilev, which was based on a patch by Bernd
Schmidt, which was based on a patch by Reid Kleckner.

On 8 August 2017 at 12:12, Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Tue Aug  8 12:12:28 2017
> New Revision: 310401
>
> URL: http://llvm.org/viewvc/llvm-project?rev=310401=rev
> Log:
> PR19668, PR23034: Fix handling of move constructors and deleted copy
> constructors when deciding whether classes should be passed indirectly.
>
> This fixes ABI differences between Clang and GCC:
>
>  * Previously, Clang ignored the move constructor when making this
>determination. It now takes the move constructor into account, per
>https://github.com/itanium-cxx-abi/cxx-abi/pull/17 (this change may
>seem recent, but the ABI change was agreed on the Itanium C++ ABI
>list a long time ago).
>
>  * Previously, Clang's behavior when the copy constructor was deleted
>was unstable -- depending on whether the lazy declaration of the
>copy constructor had been triggered, you might get different behavior.
>We now eagerly declare the copy constructor whenever its deletedness
>is unclear, and ignore deleted copy/move constructors when looking for
>a trivial such constructor.
>
> This also fixes an ABI difference between Clang and MSVC:
>
>  * If the copy constructor would be implicitly deleted (but has not been
>lazily declared yet), for instance because the class has an rvalue
>reference member, we would pass it directly. We now pass such a class
>indirectly, matching MSVC.
>
> Modified:
> cfe/trunk/include/clang/AST/DeclCXX.h
> cfe/trunk/lib/AST/ASTImporter.cpp
> cfe/trunk/lib/AST/DeclCXX.cpp
> cfe/trunk/lib/CodeGen/CGCXXABI.cpp
> cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
> cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
> cfe/trunk/lib/Serialization/ASTWriter.cpp
> cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp
> cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
>
> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/AST/DeclCXX.h?rev=310401=310400=310401=diff
> 
> ==
> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
> +++ cfe/trunk/include/clang/AST/DeclCXX.h Tue Aug  8 12:12:28 2017
> @@ -375,6 +375,7 @@ class CXXRecordDecl : public RecordDecl
>  /// \brief These flags are \c true if a defaulted corresponding
> special
>  /// member can't be fully analyzed without performing overload
> resolution.
>  /// @{
> +unsigned NeedOverloadResolutionForCopyConstructor : 1;
>  unsigned NeedOverloadResolutionForMoveConstructor : 1;
>  unsigned NeedOverloadResolutionForMoveAssignment : 1;
>  unsigned NeedOverloadResolutionForDestructor : 1;
> @@ -383,6 +384,7 @@ class CXXRecordDecl : public RecordDecl
>  /// \brief These flags are \c true if an implicit defaulted
> corresponding
>  /// special member would be defined as deleted.
>  /// @{
> +unsigned DefaultedCopyConstructorIsDeleted : 1;
>  unsigned DefaultedMoveConstructorIsDeleted : 1;
>  unsigned DefaultedMoveAssignmentIsDeleted : 1;
>  unsigned DefaultedDestructorIsDeleted : 1;
> @@ -415,6 +417,12 @@ class CXXRecordDecl : public RecordDecl
>  /// constructor.
>  unsigned HasDefaultedDefaultConstructor : 1;
>
> +/// \brief True if this class can be passed in a
> non-address-preserving
> +/// fashion (such as in registers) according to the C++ language
> rules.
> +/// This does not imply anything about how the ABI in use will
> actually
> +/// pass an object of this class.
> +unsigned CanPassInRegisters : 1;
> +
>  /// \brief True if a defaulted default constructor for this class
> would
>  /// be constexpr.
>  unsigned DefaultedDefaultConstructorIsConstexpr : 1;
> @@ -811,18 +819,50 @@ public:
>  return data().FirstFriend.isValid();
>}
>
> +  /// \brief \c true if a defaulted copy constructor for this class would
> be
> +  /// deleted.
> +  bool defaultedCopyConstructorIsDeleted() const {
> +assert((!needsOverloadResolutionForCopyConstructor() ||
> +(data().DeclaredSpecialMembers & SMF_CopyConstructor)) &&
> +   "this property has not yet been computed by Sema");
> +return data().DefaultedCopyConstructorIsDeleted;
> +  }
> +
> +  /// \brief \c true if a defaulted move constructor for this class would
> be
> +  /// deleted.
> +  bool defaultedMoveConstructorIsDeleted() const {
> +assert((!needsOverloadResolutionForMoveConstructor() ||
> +(data().DeclaredSpecialMembers & SMF_MoveConstructor)) &&
> +   "this property has not yet been computed by Sema");
> +return data().DefaultedMoveConstructorIsDeleted;
> +  }
> +
>