This is an archive of the discontinued LLVM Phabricator instance.

[asan] Remove check for stack size
ClosedPublic

Authored by Hahnfeld on Jun 30 2017, 5:14 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

Hahnfeld created this revision.Jun 30 2017, 5:14 AM
alekseyshl edited edge metadata.Jun 30 2017, 10:04 AM

What's the stack size on CentOS? What was the point of the original check?

vitalybuka edited edge metadata.Jun 30 2017, 10:32 AM
vitalybuka added a subscriber: morehouse.

Matt also hit this on Unbuntu with gcc as a host compiler.

Well, would be nice to know the size of the stack there too. I'm curious what was the reason to check that stack size is less than 256MB. Default max_uar_stack_size_log is 20, so it's 1MB of stack size and I don't see where else stack_size is used.

Hahnfeld edited the summary of this revision. (Show Details)Jul 1 2017, 3:52 AM

What's the stack size on CentOS? What was the point of the original check?

The error is as follows:

"((stack_size)) <= ((0x10000000))" (0x40000000, 0x10000000)

Please note that I have ulimit -s unlimited because I often had problems with Fortran applications that required large stack sizes. With ulimit -s 4096, the problem also goes away.
However, I don't understand this check either, so I'd say to remove it as it causes problems.

alekseyshl accepted this revision.Jul 3 2017, 9:48 AM

What's the stack size on CentOS? What was the point of the original check?

The error is as follows:

"((stack_size)) <= ((0x10000000))" (0x40000000, 0x10000000)

Please note that I have ulimit -s unlimited because I often had problems with Fortran applications that required large stack sizes. With ulimit -s 4096, the problem also goes away.
However, I don't understand this check either, so I'd say to remove it as it causes problems.

I'm ok with removing this check, we can always bring it back (with proper comment this time).

Kostya, what exactly this check was for?

This revision is now accepted and ready to land.Jul 3 2017, 9:48 AM
This revision was automatically updated to reflect the committed changes.
kcc edited edge metadata.Jul 12 2017, 11:04 AM

This check is a sanity check, please keep it, but feel free to change the constant to e.g. 0x100000000 (16x increase)

In D34876#806869, @kcc wrote:

This check is a sanity check, please keep it, but feel free to change the constant to e.g. 0x100000000 (16x increase)

Sanity check in what sense? If we can increase it arbitrarily, it's not a check anymore IMHO

kcc added a comment.Jul 13 2017, 1:36 PM
In D34876#806869, @kcc wrote:

This check is a sanity check, please keep it, but feel free to change the constant to e.g. 0x100000000 (16x increase)

Sanity check in what sense? If we can increase it arbitrarily, it's not a check anymore IMHO

It's a sanity check that asan has computed the stack size correctly, and that it doesn't think the stack is e.g. 42 Gb