onetrueawk / awk

One true awk
Other
1.96k stars 156 forks source link

Function argument fixes #198

Open mpinjr opened 9 months ago

mpinjr commented 9 months ago

Fix argument counting error which cuts the argument capacity in half. Fix conversion of uninitialized function arguments into arrays. Plug some leaks and eliminate some uses-after-free.

Hope everyone is doing well, Miguel

mpinjr commented 9 months ago

I recommend against using that last commit (b5e23fd). There are a few things about it that require tidying up. Also, what I had thought of as an unrelated branch of work on temp cells may intersect with this. I would like a little time to explore that.

The anonymous arg cells should be okay for now. There are already anonymous cells (CTEMP mostly) floating around when compile_time == RUNNING. If printing a NULL nval causes a crash on a non-BSD/Linux system, it would have served to uncover a print statement that needs fixing.

As @plan9 mentioned, there's already plenty to digest here. If we're in agreement, should I force push to my branch a reset to the previous HEAD or just leave things as they are? (I know how to use git, but github not so much).

Take care, Miguel

millert commented 9 months ago

@mpinjr I think it is fine to go ahead with the anonymous arg cells. I can work on a PR to add missing NN wrapping if you'd like.

mpinjr commented 9 months ago

@millert Thanks, but it's possible that it would be a waste of time. Here's what I have in mind...

As you know, it's not safe to call execute if there exists an unhandled cell from an earlier execute. To avoid use-after-free (and other) bugs, whatever is needed from the first execute's cell must be obtained before the next execute begins.

It occurred to me that if this discipline were applied consistently, we would only need one CTEMP cell. That cell could be statically allocated with access to it guarded by a boolean. An attempt to acquire it while busy would be a fatal error.

(Like the function calling work, this is one of the ideas I was exploring last year.)

So, if function arguments were named and if there were only a single tempcell, the implementation would be left with just 9 anonymous cells (defined near the top of run.c). Those could be named and then wrapping nval would be undesirable (as it would prevent sensitive platforms from denouncing the existence of an anonymous cell that should not be).