Skip to content

Commit

Permalink
Avoid using Deltas to insert text
Browse files Browse the repository at this point in the history
Deltas were designed to replace text, not inserting text.  This may be
a source of bugs, so we only register the change we did into the
FileEntryMap and pass the change directly to clang's Rewriter.

Signed-off-by: Giuliano Belinassi <gbelinassi@suse.de>
  • Loading branch information
giulianobelinassi committed Jun 6, 2024
1 parent 8b7caed commit 85fb745
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 31 deletions.
80 changes: 50 additions & 30 deletions libcextract/SymbolExternalizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -308,42 +308,62 @@ void TextModifications::Solve(void)
DeltaList = new_arr;
}

void TextModifications::Commit(void)
bool TextModifications::Insert_Into_FileEntryMap(const SourceLocation &loc)
{
Solve();
int n = DeltaList.size();
/* Register the changed FileID for retrieving the buffer later. */
FileID begin_id = SM.getFileID(loc);

for (int i = 0; i < n; i++) {
// Commit Change.
Delta &a = DeltaList[i];
/* Insert into the list of FileIDs. */
const FileEntry *fentry = SM.getFileEntryForID(begin_id);

/* There are some cases where the fentry is known to return NULL. Check if
those are the cases we already acknownledged. */
if (fentry == nullptr) {
PresumedLoc ploc = SM.getPresumedLoc(loc);
if (ploc.getFilename() == StringRef("<command line>")) {
/* Locations comming from the command line can be ignored. */
return false;
}

/* Crash with assertion. */
assert(fentry && "FileEntry is NULL on a non-acknowledged case");
}

/* Insert the FileEntry if we don't have one. */
if (FileEntryMap.find(fentry) == FileEntryMap.end()) {
/* Insert it. */
FileEntryMap[fentry] = begin_id;
return true;
}

/* Nothing was inserted. */
return false;
}

bool TextModifications::Insert_Into_FileEntryMap(const SourceRange &range)
{
/* Register the changed FileID for retrieving the buffer later. */
FileID begin_id = SM.getFileID(a.ToChange.getBegin());
FileID end_id = SM.getFileID(a.ToChange.getEnd());
FileID begin_id = SM.getFileID(range.getBegin());
FileID end_id = SM.getFileID(range.getEnd());

/* Ensure that the fileIDs are equal. */
assert(begin_id == end_id);

/* Insert into the list of FileIDs. */
const FileEntry *fentry = SM.getFileEntryForID(begin_id);
return Insert_Into_FileEntryMap(range.getBegin());
}

/* There are some cases where the fentry is known to return NULL. Check if
those are the cases we already acknownledged. */
if (fentry == nullptr) {
PresumedLoc ploc = SM.getPresumedLoc(a.ToChange.getBegin());
if (ploc.getFilename() == StringRef("<command line>")) {
/* Locations comming from the command line can be ignored. */
continue;
}
void TextModifications::Commit(void)
{
Solve();
int n = DeltaList.size();

/* Crash with assertion. */
assert(fentry && "FileEntry is NULL on a non-acknowledged case");
}
/* Insert the FileEntry if we don't have one. */
if (FileEntryMap.find(fentry) == FileEntryMap.end()) {
/* Insert it. */
FileEntryMap[fentry] = begin_id;
}
for (int i = 0; i < n; i++) {
// Commit Change.
Delta &a = DeltaList[i];

/* Try to insert into the FileEntryMap for commiting the change to the buffer
later. */
Insert_Into_FileEntryMap(a.ToChange);

/* Get the text we want to change. We only do this to know its length. */
StringRef source_text = Lexer::getSourceText(CharSourceRange::getCharRange(
Expand Down Expand Up @@ -533,9 +553,10 @@ void SymbolExternalizer::Remove_Text(const SourceRange &range, int prio)
Replace_Text(range, "", prio);
}

void SymbolExternalizer::Insert_Text(const SourceRange &range, StringRef text, int prio)
void SymbolExternalizer::Insert_Text(const SourceLocation &loc, StringRef text)
{
Replace_Text(range, text, prio);
TM.Insert_Into_FileEntryMap(loc);
TM.Get_Rewriter().InsertText(loc, text);
}

VarDecl *SymbolExternalizer::Create_Externalized_Var(DeclaratorDecl *decl, const std::string &name)
Expand Down Expand Up @@ -939,8 +960,7 @@ void SymbolExternalizer::Externalize_Symbols(std::vector<std::string> const &to_
SourceManager &sm = AST->getSourceManager();
FileID fi = sm.getMainFileID();
SourceLocation sl = sm.getLocForStartOfFile(fi);
SourceRange sr(sl, sl);
Insert_Text(sr, "#include <linux/livepatch.h>\n", 1000);
Insert_Text(sl, "#include <linux/livepatch.h>\n");
}

for (const std::string &to_externalize : to_externalize_array) {
Expand Down
6 changes: 5 additions & 1 deletion libcextract/SymbolExternalizer.hh
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@ class TextModifications
return FileEntryMap;
}

/* Insert into the FileEntryMap hash. */
bool Insert_Into_FileEntryMap(const SourceLocation &loc);
bool Insert_Into_FileEntryMap(const SourceRange &range);

/* Dump, for debugging purposes. */
void Dump(unsigned, const Delta &a);

Expand Down Expand Up @@ -271,7 +275,7 @@ class SymbolExternalizer
used in case that there are two replacements to the same piece of text. */
void Replace_Text(const SourceRange &range, StringRef new_name, int priority);
void Remove_Text(const SourceRange &range, int priority);
void Insert_Text(const SourceRange &range, StringRef text, int priority);
void Insert_Text(const SourceLocation &range, StringRef text);

/** AST in analysis. */
ASTUnit *AST;
Expand Down

0 comments on commit 85fb745

Please sign in to comment.