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

Add a set of comments and question on PDG construction #1092

Closed
wants to merge 1 commit into from

Conversation

ahomescu
Copy link
Contributor

I wrote my questions on the PDG builder algorithm in the form of comments. It might be useful to answer (some of) these and leave them in as comments, for future reference.

@ahomescu ahomescu requested review from kkysen and fw-immunant May 10, 2024 08:07
@ahomescu ahomescu self-assigned this May 10, 2024
@ahomescu ahomescu force-pushed the ahomescu/pdg_comments branch from 59e0e61 to 5eb68f9 Compare May 11, 2024 05:24
@ahomescu ahomescu requested a review from rinon May 13, 2024 21:07
Comment on lines +47 to 48
// Question: this takes a pointer as input and produces another?
Field(ptr, ..) => ptr,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Question: this takes a pointer as input and produces another?

That's what I would expect. In the static analysis, we have a similar notion of PlaceElem::Field as a "pointer projection" that takes a pointer to the struct and produces a pointer to the field. This is similar to (some uses of) offset, which takes a pointer to (the first element of) an array and produces a pointer to an element, and also similar to LLVM's GetElementPtr operation.

Comment on lines +63 to 64
// Question: same, is the base pointer enough?
Offset(ptr, _, _) => ptr,
Copy link
Collaborator

Choose a reason for hiding this comment

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

is the base pointer enough?

At one point, we planned to distinguish forward offsets (OFFSET_ADD) from backward ones (OFFSET_SUB). Offsets that statically only go forward can be implemented on slices with a subslice operation, but offsets that might go backward need a different type that tracks how far back it can go and remain in bounds.

Comment on lines +173 to +176
// Question: is "provenance" exactly tied to the memory address of this pointer?
// Why don't we just use that for everything?
// Answer attempt: provenance only covers the original
// source of the pointer, and not any copies like LoadValue
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't have an answer for this case, but in general it's possible for two pointers to have the same address but different provenance. In C:

int x;
int y;
int* p = &x + 1;
int* q = &y;

It's possible that p == q, but q has &y's provenance and is legal to dereference, while p has &x's provenance and dereferencing it is UB (it's out of bounds for the object it points to).

@ahomescu ahomescu closed this Jun 7, 2024
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