Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copy function debug name onto stack for crash dumps #159

Merged
merged 4 commits into from Oct 10, 2018

Conversation

@asherkin
Copy link
Member

asherkin commented Oct 10, 2017

This is a useful hack I've had sitting in my queue for a while.

Copy the to-be-invoked function's debug name into the ScriptedInvoker::Invoke stack frame so it is always available in minidumps even without heap information.

This will hopefully make it easier to track down reproduction steps for plugin-related crashes.

@asherkin asherkin requested a review from dvander Oct 10, 2017
Copy link
Member

dvander left a comment

I think a reasonable, statically-sized buffer is better. alloca() has Problems(TM).

asherkin added 2 commits Feb 3, 2018
Also avoid higher-level compiler optimisations removing the name
@asherkin

This comment has been minimized.

Copy link
Member Author

asherkin commented Feb 3, 2018

I've played with a few variations in https://godbolt.org/, I think alloca remains the best option all-round - the location and behaviour is a lot more predictable with compiler optimisations, and choosing a reasonable buffer size to avoid wasting stack space in all cases without truncating is a PITA.

@KyleSanderson

This comment has been minimized.

Copy link
Member

KyleSanderson commented Feb 3, 2018

http://man7.org/linux/man-pages/man3/alloca.3.html

`There is no error indication if the stack frame cannot be extended.
(However, after a failed allocation, the program is likely to receive
a SIGSEGV signal if it attempts to access the unallocated space.)

   On many systems alloca() cannot be used inside the list of arguments
   of a function call, because the stack space reserved by alloca()
   would appear on the stack in the middle of the space for the function
   arguments.`

`This function is not in POSIX.1.

   There is evidence that the alloca() function appeared in 32V, PWB,
   PWB.2, 3BSD, and 4BSD.  There is a man page for it in 4.3BSD.  Linux
   uses the GNU version.`

I would avoid alloca at all costs.

@asherkin

This comment has been minimized.

Copy link
Member Author

asherkin commented Feb 3, 2018

As would just calling the function if it was allocating a large enough local variable... that information isn't exactly useful or relevant.

The DebugName of a function is fairly well constrained (and I believe should be well under the limit reasonable to stack alloc - which MS considers to be 1kB for an individual alloc), it just isn't ideal to allocate the full size required for the 90% of cases where it is much shorter.

@asherkin asherkin merged commit 3e44acb into master Oct 10, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Headline Headline deleted the debug-name-for-crash-dumps branch Oct 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.