950545 - Fix the DMD test

Status: RESOLVED FIXED

(Core :: DMD, defect)

Description

[Nicholas Nethercote [inactive]]

The DMD test is totally busted.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Fix the dump numbering in the DMD test.

In the output of the DMD test, the dumps are numbered 1--4.  But the comments
number them 0--3.  This patch makes the comments match the output, which makes
it easier to understand what is happening.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Fix assertion failures and output mismatches in the DMD test.

There were two problems with the test.

- Compilers are getting too clever!  UseItOrLoseIt(), as formulated, wasn't
  enough, and clang (at least) was optimizing away some of the malloc() calls,
  leading to assertion failures near the end of the test.  (It's possible that
  MSVC has been doing likewise.)

  I changed it to print out the address in UseItOrLoseIt().  This clogs up the
  test output with some addresses, but that doesn't matter.

- The filtering of output wasn't aggressive enough.  There were some minor
  differences between the expected and actual outputs.  So I made the stack
  trace record filtering stronger, and I also am now filtering stack frame
  records away entirely.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Fix assertion failures and output mismatches in the DMD test.

This version fixes the stress test so it doesn't print out lots of output.
UseItOrLoseIt2 doesn't get optimized away, I suspect because it's called from a
single call site and so the compiler can't do any optimization based on the
fact that subsequent calls to malloc() cannot return the same value.

Comment 4

[Catalin Iacob]

Comment on attachment 8347866 [details] [diff] [review]
(part 2) - Fix assertion failures and output mismatches in the DMD test.

Review of attachment 8347866 [details] [diff] [review]:
-----------------------------------------------------------------

The test still doesn't work on Windows but it never did so that's for another bug.

r=me with the write issue addressed.

::: memory/replace/dmd/DMD.cpp
@@ +2300,5 @@
>  UseItOrLoseIt(void* a)
>  {
> +  char buf[64];
> +  sprintf(buf, "%p\n", a);
> +  write(1, buf, strlen(buf) + 1);

This breaks the build on Windows. Should either be _write there and <io.h> should be included (see http://msdn.microsoft.com/en-us/library/1570wh78.aspx) or better just to use fwrite.

Comment 5

[Nicholas Nethercote [inactive]]

> The test still doesn't work on Windows but it never did so that's for
> another bug.

How does it fail -- assertion failure?

Comment 6

[Catalin Iacob]

(In reply to Nicholas Nethercote [:njn] from comment #5)
> How does it fail -- assertion failure?

Just like it did in https://bugzilla.mozilla.org/show_bug.cgi?id=819839#c18.

With --enable-debug it asserts in the same place because of the same reason, with --enable-release it runs and produces a file which is not empty but with no information (0 bytes everywhere, no stack traces). Basically, it doesn't manage to hook malloc calls so it doesn't know about any allocation.

Comment 7

[Catalin Iacob]

Attachment: windows test.dmd for --enable-release

I've attached the file to make it clear what I mean by "no information".

Comment 8

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/50c8f77c74c2
https://hg.mozilla.org/integration/mozilla-inbound/rev/9db8c2398cc7

I tested on Linux and Mac.  The Windows failure is weird.  I won't pretend to understand why it ends up using the wrong malloc.

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/50c8f77c74c2
https://hg.mozilla.org/mozilla-central/rev/9db8c2398cc7