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

hydra: fix segfaults in argument parsing #5598

Merged
merged 1 commit into from
Oct 15, 2021
Merged

Conversation

hzhou
Copy link
Contributor

@hzhou hzhou commented Oct 12, 2021

Pull Request Description

Add checks for missing argument. As a TODO, we should enhance the error
reporting to make it more user friendly. But we should prevent segfault
for now at least.

Fixes #5594

Author Checklist

  • Provide Description
    Particularly focus on why, not what. Reference background, issues, test failures, xfail entries, etc.
  • Commits Follow Good Practice
    Commits are self-contained and do not do two things at once.
    Commit message is of the form: module: short description
    Commit message explains what's in the commit.
  • Passes All Tests
    Whitespace checker. Warnings test. Additional tests via comments.
  • Contribution Agreement
    For non-Argonne authors, check contribution agreement.
    If necessary, request an explicit comment from your companies PR approval manager.

@hzhou
Copy link
Contributor Author

hzhou commented Oct 12, 2021

test:mpich/ch3/tcp

@hzhou hzhou requested a review from raffenet October 12, 2021 14:37
@hzhou
Copy link
Contributor Author

hzhou commented Oct 12, 2021

The fixed behavior:

$ mpirun
[mpiexec@Tiger] HYDU_create_proxy_list (utils/alloc/alloc.c:436): Missing executable.
[mpiexec@Tiger] main (ui/mpich/mpiexec.c:263): unable to create proxy list
$ mpirun -n
[mpiexec@Tiger] np_fn (ui/mpich/utils.c:788): missing command line argument, add -h for help
[mpiexec@Tiger] match_arg (utils/args/args.c:156): match handler returned error
[mpiexec@Tiger] HYDU_parse_array (utils/args/args.c:178): argument matching returned error
[mpiexec@Tiger] parse_args (ui/mpich/utils.c:1639): error parsing input array
[mpiexec@Tiger] HYD_uii_mpx_get_parameters (ui/mpich/utils.c:1691): unable to parse user arguments
[mpiexec@Tiger] main (ui/mpich/mpiexec.c:127): error parsing parameters

@@ -777,6 +785,7 @@ static HYD_status np_fn(char *arg, char ***argv)
status = get_current_exec(&exec);
HYDU_ERR_POP(status, "get_current_exec returned error\n");

ASSERT_ARGV;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the idea to make this a macro so it can be added to other argument parsing functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

Add checks for missing argument. As a TODO, we should enhance the error
reporting to make it more user friendly. But we should prevent segfault
for now at least.
@hzhou hzhou merged commit f5d1382 into pmodels:main Oct 15, 2021
@hzhou hzhou deleted the 2110_hydra_args branch October 15, 2021 20:41
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.

hydra: segmentation fault with incorrect arguments
2 participants