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

y_hooks funky parameter counts #57

Closed
Y-Less opened this issue Jan 5, 2018 · 13 comments
Closed

y_hooks funky parameter counts #57

Y-Less opened this issue Jan 5, 2018 · 13 comments

Comments

@Y-Less
Copy link
Contributor

Y-Less commented Jan 5, 2018

y_hooks does some unusual manipulation of parameter counts - using a negative number so that RETN actually puts data back ON to the stack instead of removing it. When there is a crash, it means this outputs things like:

[00:52:03] [debug] AMX backtrace:
[00:52:03] [debug] #0 00000005 in OnPlayerConnect (playerid=6, ... <1073741822 arguments>) at <unknown file>:0
[00:52:03] [debug] #1 00016294 in public OnPlayerConnect () at c:\server\pawno\include\YSI\..\YSI_Data\..\YSI_Coding\..\YSI_Internal\y_cgen.inc:115

This is not a bug in crashdetect - it is correctly reporting what it is told. However, it would be nice if it knew of this quirk and could walk the stack a bit more to get the real parameter count. If the parameter count is negative, the parameter count for the current function will be equal to the parameter count of the previous function (which is a small shim generated by y_hooks). Unless, of course, there is a different bug that is just corrupting the parameter count and the strange number is just wrong and nothing to do with y_hooks, in which case this adaptation won't help.

@Zeex
Copy link
Owner

Zeex commented Jan 12, 2018

Can you add a repro?

@Zeex
Copy link
Owner

Zeex commented Aug 23, 2018

So... do you have an example script?

@Y-Less
Copy link
Contributor Author

Y-Less commented Aug 24, 2018

#include <a_samp>
#include <YSI_Coding\y_hooks>

hook FakeCallback(c[32], d)
{
	printf("%d %d", BAD_numargs(), numargs());
	// Force a crash.
	c[d] = 42;
}

main()
{
	new arr[32];
	CallLocalFunction("FakeCallback", "ai", arr, 100);
}

Gives:

1073741823 2
[debug] Run time error 5: "Invalid memory access"
[debug] AMX backtrace:
[debug] #0 00011cdc in ?? (27088, 100, 0, 0, 8, 27088, 100, 16, 27020, 27072, ... <1073741813 arguments>) from crashdetectcrash.amx
[debug] #1 0000b6d4 in public FakeCallback (27088, 100) from crashdetectcrash.amx
[debug] #2 native CallLocalFunction () from samp-server.exe
[debug] #3 00011d48 in main () from crashdetectcrash.amx

@Y-Less
Copy link
Contributor Author

Y-Less commented Aug 24, 2018

Although that doesn't actually look right, I'm not sure why it is reporting 1073741813. y_hooks pushes -4 as the parameter count (a hack to actually put data back on the stack instead of removing it), and numargs (I thought) just did return *(frm + 8) / 4. I've confirmed that is -4 as expected, but I'm not sure why I'm getting 1073741823 instead of 1073741813.

It took me a long time looking at this, but my code is reporting 1073741823 as expected, crashdetect was reporting 1073741813 - I just realised that is because it shows 10 and the number is the remaining ones. So this is correct(ish).

@Zeex
Copy link
Owner

Zeex commented Aug 24, 2018

Thanks, I will look into it 👍

Zeex added a commit that referenced this issue Aug 30, 2018
Print first 10 argumenmts regardless of whether they are variadic or
named (fixed).

Before this only named arguments used to be printed with debug info
loaded, but without debug info up to 10 arguments were printed,
including variadic arguments, which can useful to see sometimes. So
fix this unfairness by always printing first 10 arguments, with or
without debug info.

This is related to #57.
@Zeex
Copy link
Owner

Zeex commented Aug 30, 2018

I think it should work better now after the recent commits, though maybe not ideal

before:

[debug] Run time error 4: "Array index out of bounds"
[debug]  Attempted to read/write array element at index 100 in array of size 32
[debug] AMX backtrace:
[debug] #0 0002e7f0 in ?? (c[32]=@00005d84 "abc", d=100, ... <1073741821 more arguments>) at ysi_stacktrace_bug.pwn:8
[debug] #1 00008118 in public FakeCallback (... <2 more arguments>) at include/YSI/\YSI_Coding\..\YSI_Internal\y_cgen.inc:30
[debug] Run time error 5: "Invalid memory access"
[debug] AMX backtrace:
[debug] #0 00025d84 in ?? (23940, 100, 0, 0, 8, 23940, 100, 16, 23872, 23924, ... <1073741813 more arguments>) in ysi_stacktrace_bug.amx
[debug] #1 00006348 in public FakeCallback (23940, 100) in ysi_stacktrace_bug.amx

after:

[debug] Run time error 4: "Array index out of bounds"
[debug]  Attempted to read/write array element at index 100 in array of size 32
[debug] AMX backtrace:
[debug] #0 0002e7f0 in @yH_FakeCallback@001 (c[32]=@00005d84 "abc", d=100) at ysi_stacktrace_bug.pwn:8
[debug] #1 00008118 in public FakeCallback (23940, 100) at include/YSI/\YSI_Coding\..\YSI_Internal\y_cgen.inc:30
[debug] Run time error 5: "Invalid memory access"
[debug] AMX backtrace:
[debug] #0 00025d84 in ?? (23940, 100) in ysi_stacktrace_bug.amx
[debug] #1 00006348 in public FakeCallback (23940, 100) in ysi_stacktrace_bug.amx

@Y-Less
Copy link
Contributor Author

Y-Less commented Aug 30, 2018

Looks correct to me. What's not ideal about it?

@Zeex
Copy link
Owner

Zeex commented Aug 30, 2018

I mean this:

FakeCallback (23940, 100) <- can't display parameter names here

I guess it's fine, they are shown in the frame above anyway

@Zeex
Copy link
Owner

Zeex commented Aug 30, 2018

Functions that begin with @ are publics, right? It looks like @yH_FakeCallback@001 is not found in the publics table, not sure if it's normal

@Y-Less
Copy link
Contributor Author

Y-Less commented Aug 30, 2018

Ahh I see. Well that's a runtime generated function, so it makes sense that it has no debug information. So this is brilliant thanks.

@Zeex
Copy link
Owner

Zeex commented Aug 30, 2018

OK.

@Zeex Zeex closed this as completed Aug 30, 2018
@Y-Less
Copy link
Contributor Author

Y-Less commented Aug 30, 2018

Yes they are publics, but those ones get removed from the publics table to free up space for new generated names.

@Y-Less
Copy link
Contributor Author

Y-Less commented Aug 30, 2018

Basically, y_hooks does all sorts of crazy things to the assembly to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants