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

Modify context generation slightly (xrefs) #272

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Conversation

trashvisor
Copy link
Contributor

Truncate xrefs, so that the source code of the entire function(s) xref'ing the function under test is not included.

Additionally, modify context generation so we talk about lines where the code is xref'd, as opposed to functions.

@trashvisor trashvisor requested a review from DonggeLiu May 15, 2024 00:21
# If it was, truncate it.
if line_index != -1:
start = start if line_index <= 10 else line_index - 10
end = end if line_index >= end - 10 else line_index + 10
Copy link
Collaborator

@DonggeLiu DonggeLiu May 15, 2024

Choose a reason for hiding this comment

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

nit: lines[1:2] will not include line index 2.

start = max(0, line_index - 10) 
end = min(len(lines), line_index+10)

@DonggeLiu
Copy link
Collaborator

/gcbrun request_pr_exp.py -n dg-272

@DonggeLiu
Copy link
Collaborator

@trashvisor Could you please check if the report:

  1. Has any unexpected behavior (e.g., failed to include certain xrefs)
  2. Significant improvements / regressions?
    https://llm-exp.oss-fuzz.com/Result-reports/ofg-pr/2024-05-15-272-dg-272-comparison/index.html
    Thanks!

if not, I reckon this is ready to merge.

@oliverchang
Copy link
Collaborator

/gcbrun request_pr_exp.py -n oc-272

@DonggeLiu
Copy link
Collaborator

DonggeLiu commented May 22, 2024

A silly question:
What is the intention of this block in the prompt?
Is it to mitigate the 'undefined function' error?

I noticed that it adds the function declaration, which is the same as the function signature added earlier.

Related:

must_insert=context_info['decl'],

@trashvisor
Copy link
Contributor Author

A silly question: What is the intention of this block in the prompt? Is it to mitigate the 'undefined function' error?

I noticed that it adds the function declaration, which is the same as the function signature added earlier.

Related:

must_insert=context_info['decl'],

It was originally intended to solve:

  • Not having the appropriate header file for the function declaration (so we would just embed the function decl itself without a header and things might work).
  • Compatibility with c programs, as an extern "C" would be added to solve linkage issues.

Should this block be removed or some templating logic added?

@DonggeLiu
Copy link
Collaborator

It was originally intended to solve:

  • Not having the appropriate header file for the function declaration (so we would just embed the function decl itself without a header and things might work).

Ah, thanks for the explanation!

I doubt this will help LLM get the correct header file, though:

  1. We already have the signature in the prompt, so this doesn't add new information.
  2. LLM is not good at searching for the corresponding header file given the function signature, it can only guess.

But don't worry; AdaLogic will look into finding the correct header file in a more programmatic way.

  • Compatibility with c programs, as an extern "C" would be added to solve linkage issues.

In this case, maybe:

  1. Make the instruction more explicit, e.g.,
Include the following extern C linkage specification in the fuzz target, because the function-under-test is defined in C and the fuzz target is written in C++:
extern ''C" <function-signature>
  1. Only add this code if the project-under-test is in C.

Would this sound reasonable? Thanks : )

@DonggeLiu DonggeLiu marked this pull request as draft September 4, 2024 01:05
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.

3 participants