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

Supply Pallene Tracer Error Handler Function #25

Merged
merged 2 commits into from
Aug 22, 2024
Merged

Conversation

singul4ri7y
Copy link
Collaborator

Changelog:

  • Supply the error handler function to be used against xpcall.

Copy link
Member

@hugomg hugomg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if assigning to a global variable might cause trouble with Lua environments that don't allow assigning to globals. For example, the strict.lua that sets the __newindex global metatable...

pt-lua.c Show resolved Hide resolved
@singul4ri7y
Copy link
Collaborator Author

I think, here setting the global variable won't cause us any issue. Because even if the script manages to override the __newindex metamethod, we will be setting the global variable prior to calling the main chunk. Hence, setting global before the metamethod override.

@hugomg
Copy link
Member

hugomg commented Aug 21, 2024

I suppose that would depend on the order that the modules are required. If they require strict.lua before they run the tracer initializer, then it could cause the trouble?

@singul4ri7y
Copy link
Collaborator Author

singul4ri7y commented Aug 21, 2024

Actually the function is not supplied with tracer initializer. It is being supplied by setting Lua global prior calling the MAIN chunk, before any of the module is initialized, in the main function of pt-lua. So, we are bulletproof here.

int main (int argc, char **argv) {
  int status, result;
  lua_State *L = luaL_newstate();  /* create state */
  if (L == NULL) {
    l_message(argv[0], "cannot create state: not enough memory");
    return EXIT_FAILURE;
  }
  lua_gc(L, LUA_GCSTOP);  /* stop GC while building state */

  /* -------- PALLENE TRACER CODE -------- */
  (void) pallene_tracer_init(L);  /* initialize pallene tracer */
  lua_pop(L, 1);  /* We do not need the finalizer object here */

  /* supply the message handler function with custom tracebacks. */
  lua_pushcfunction(L, msghandler);
  lua_setglobal(L, "pallene_tracer_errhandler");
  /* -------- PALLENE TRACER CODE END -------- */

  lua_pushcfunction(L, &pmain);  /* to call 'pmain' in protected mode */
  lua_pushinteger(L, argc);  /* 1st argument */
  lua_pushlightuserdata(L, argv); /* 2nd argument */
  status = lua_pcall(L, 2, 1, 0);  /* do the call */
  result = lua_toboolean(L, -1);  /* get result */
  report(L, status);
  lua_close(L);
  return (result && status == LUA_OK) ? EXIT_SUCCESS : EXIT_FAILURE;
}

@hugomg
Copy link
Member

hugomg commented Aug 21, 2024

Oh, that makes sense.

Does pt-lua accept "-l" command line flags and, if so, do those requires run before or after our "main"?

Can you also please add a comment explaining why this global is safe?

@singul4ri7y
Copy link
Collaborator Author

pt-lua is exact copy of lua.c. So, whatever Lua frontend can do, pt-lua can do as well. To answer your question, yes pt-lua do accept -l and the require happens after we are done setting the global as the code suggest. The pmain function is responsible for parsing the flags and we are setting the global just before calling pmain with a protected call.

Comment where? Here, or in the code explaining?

@hugomg
Copy link
Member

hugomg commented Aug 21, 2024

Comment in the code, next to where you set the global.

@singul4ri7y
Copy link
Collaborator Author

Can you please clarify to comment about what exactly? Safe/unsafe in what way?

@hugomg
Copy link
Member

hugomg commented Aug 21, 2024

Just say that it's ok to set a global variable, because no other code has run yet. No one could have messed with the global meta table.

@singul4ri7y
Copy link
Collaborator Author

Done, we can merge the PR now.

@hugomg
Copy link
Member

hugomg commented Aug 22, 2024

Since there are many PRs at the same time, please rebase them before merging. Cheers!

@singul4ri7y singul4ri7y merged commit 9aa8606 into main Aug 22, 2024
1 check passed
@singul4ri7y singul4ri7y deleted the supply-traceback-fn branch August 22, 2024 14:39
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

Successfully merging this pull request may close these issues.

2 participants