-
Notifications
You must be signed in to change notification settings - Fork 135
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
Crash Analyzer Agent #814
base: main
Are you sure you want to change the base?
Crash Analyzer Agent #814
Conversation
Thanks again for the pushing the code fixing the conflicts, @maoyixie! Before that, let me start an experiment below so that we can see its results together later : ) self.conversation_history.extend(prompt.get()) |
/gcbrun exp -n mx -ag |
@@ -2,7 +2,7 @@ | |||
# | |||
# Licensed under the Apache License, Version 2.0 (the "License"); | |||
# you may not use this file except in compliance with the License. | |||
# You may obtain a copy of the License at | |||
# You may obtain a copy of the License a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo?
with open(os.path.join(generated_project_path, 'Dockerfile'), 'a') as f: | ||
f.write('\nENV FUZZING_LANGUAGE={run_result.benchmark.language}\n' | ||
'\nRUN sed -i.bak \'1i export CFLAGS="${CFLAGS} -g"\' ' | ||
'/src/build.sh\n' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Would it be simpler to modify CFLAGS in dockerfile? E.g.,
ENV CFLAGS="${CFLAGS} -g"
? - Do we need to add
-g
toCXXFLAGS
too?
trial=self.trial) | ||
return prompt_builder.DefaultTemplateBuilder(self.llm).build([]) | ||
|
||
def _create_ossfuzz_project_with_lldb(self, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reckon this function is derived from create_ossfuzz_project()
.
Some nits:
oss_fuzz_checkout.py
is a better place for this function.
That file is designed to encapsulate and handle all OSS-Fuzz related functionalities, so that OSS-Fuzz-Gen does not have to know/consider them. Same with_prepare_project_image()
inLLDBTool
below. We plan to relocatecreate_ossfuzz_project()
too soon.- Please try to avoid code duplication.
We will really appreciate this because if we need to modifycreate_ossfuzz_project()
later, we don't have to remember to repeats the same steps for this function. For example, given your function's main task is appending lines to the Dockerfile, could we first call functioncreate_ossfuzz_project()
to create a new project, then add those new lines to the Dockerfile of the new project?
'\nCOPY agent-build.sh /src/build.sh\n' | ||
'\nENV FUZZING_LANGUAGE={run_result.benchmark.language}\n' | ||
'\nRUN sed -i.bak \'1i export CFLAGS="${CFLAGS} -g"\' /src/build.sh\n' | ||
'\nRUN apt-get update && apt-get install -y lldb\n') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reckon this is another code block we can remove if we first call create_ossfuzz_project()
to create a new project, and then add these lines to the Dockerfile in the new project.
We don't have to worry about agent-build.sh
in that case.
for command in self._parse_tags(response, 'bash'): | ||
prompt_text += self._format_bash_execution_result( | ||
tool.execute(command), previous_prompt=prompt) + '\n' | ||
prompt.add_problem(prompt_text) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reckon you overwrite _container_handle_bash_command()
because you want to call add_problem()
instead of append()
for OpenAIPrompt
.
Would it be simpler to implement the append()
function in OpenAIPrompt
so that we don't have to overwrite this function?
This will make the code more transparent between different models and largely lower the complexity when we read/modify the code in the future.
For example:
def append(self, text: str, role:str='user') -> None:
"""Constructs the prompt problem in the required format."""
self._prompt.append({
'role': role,
'content': text,
})
Or append the text to an existing role-content pair, whichever is better.
def _prepare_project_image(self) -> str: | ||
"""Prepares the project's OSS-Fuzz docker image and returns the image name. | ||
""" | ||
image_name = f'gcr.io/oss-fuzz/{self.project}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better to use gcr.io/oss-fuzz/{self.project}-lldb
to distinguish it from the normal image without lldb
?
logger.info('Successfully build project image for %s', self.project) | ||
return image_name | ||
except sp.CalledProcessError: | ||
logger.info('Failed to build image for %s', self.project) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, would it be clearer to put this in oss_fuzz_checkout
, or reuse some of its code?
Ideally, OSS-Fuzz-Gen, particularly agents, don't have to know OSS-Fuzz details.
|
||
def _execute_command(self, | ||
command: list[str], | ||
in_container: bool = False) -> sp.CompletedProcess: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these functions are the same as _execute_command
and _execute_command_in_container
, could you make lldb_tool
inherit from ProjectContainerTool
so that we don't have to repeat them?
result.stderr) | ||
return result | ||
|
||
def _start_docker_container(self) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we reuse
oss-fuzz-gen/tool/container_tool.py
Line 108 in f08dd92
def _start_docker_container(self) -> str: |
process.args = command | ||
return process | ||
|
||
def terminate(self) -> bool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reuse or inherit from ProjectContainerTool
if possible : )
Hi @maoyixie, the code looks good in general, I've left some comments above, please take a look at your convenience : ) |
This PR mainly implements a crash analyzer that can interact with LLDB in the multi-agent framework, and supports GPT. In addition, this PR attempts to fix the problem of not replacing the fuzz target and build script. This PR is under testing. The main logic is no longer changing, and minor bugs are being fixed.
TODO:
Optimize the process of agent interaction with LLDB.
Solve the problem of missing debugging information for some projects.
Try to add LLM-based static methods to enhance the crash analyzer.