[clang] [clang] Enforce 1-based indexing for command line source locations (PR #139457)

Sun May 11 12:10:22 PDT 2025

naveen-seth wrote:

> Thanks for working on that. I think it might better to do that check where `ParsedSourceLocation::FromString` is called, so that we can have a proper front-end diagnostics for it (search for `err_fe_invalid_code_complete_file` and `OPT_code_completion_at`)

Thank you for the quick review! :)

I added the check in `ParsedSourceLocation::FromString` because it also helps catch the same crash for callers of `ParsedSourceRange::FromString`, which also uses `ParsedSourceLocation::FromString`.

For example, the following would also crash (hitting the same assert as the one this PR fixes)

```bash
touch empty.cpp
clang-refactor local-rename -selection=input.cpp:1:1-1:0 -new-name=test -v input.cpp 
```
<details>
<summary> Full crash log</summary>

```
Error while trying to load a compilation database:
Could not auto-detect compilation database for file "input.cpp"
No compilation database found in /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-debug-env/debugging-139375 or any parent directory
fixed-compilation-database: Error while opening fixed database: No such file or directory
json-compilation-database: Error while opening JSON database: No such file or directory
Running without flags.
clang-refactor: /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/lib/Basic/SourceManager.cpp:1666: SourceLocation clang::SourceManager::translateLineCol(FileID, unsigned int, unsigned int) const: Assertion `Line && Col && "Line and column should start from 1!"' failed.
 #0 0x0000562e7ef658cd llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/llvm/lib/Support/Unix/Signals.inc:804:11
 #1 0x0000562e7ef65e0b PrintStackTraceSignalHandler(void*) /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/llvm/lib/Support/Unix/Signals.inc:888:1
 #2 0x0000562e7ef63f2f llvm::sys::RunSignalHandlers() /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/llvm/lib/Support/Signals.cpp:105:5
 #3 0x0000562e7ef664e9 SignalHandler(int, siginfo_t*, void*) /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/llvm/lib/Support/Unix/Signals.inc:418:7
 #4 0x00007f6311a5e250 (/lib/x86_64-linux-gnu/libc.so.6+0x45250)
 #5 0x00007f6311abcf1c pthread_kill (/lib/x86_64-linux-gnu/libc.so.6+0xa3f1c)
 #6 0x00007f6311a5e19e raise (/lib/x86_64-linux-gnu/libc.so.6+0x4519e)
 #7 0x00007f6311a41902 abort (/lib/x86_64-linux-gnu/libc.so.6+0x28902)
 #8 0x00007f6311a4181e (/lib/x86_64-linux-gnu/libc.so.6+0x2881e)
 #9 0x00007f6311a547c7 (/lib/x86_64-linux-gnu/libc.so.6+0x3b7c7)
#10 0x0000562e7fbee816 clang::SourceManager::translateLineCol(clang::FileID, unsigned int, unsigned int) const /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/lib/Basic/SourceManager.cpp:1668:11
#11 0x0000562e7ed5dc34 (anonymous namespace)::SourceRangeSelectionArgument::forAllRanges(clang::SourceManager const&, llvm::function_ref<void (clang::SourceRange)>) /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/tools/clang-refactor/ClangRefactor.cpp:131:12
#12 0x0000562e7ed5eb2b (anonymous namespace)::ClangRefactorTool::callback(clang::ASTContext&) /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/tools/clang-refactor/ClangRefactor.cpp:418:11
#13 0x0000562e7ed5e840 (anonymous namespace)::ClangRefactorTool::getFrontendActionFactory()::'lambda'(clang::ASTContext&)::operator()(clang::ASTContext&) const /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/tools/clang-refactor/ClangRefactor.cpp:475:35
#14 0x0000562e7ed5e80d void std::__invoke_impl<void, (anonymous namespace)::ClangRefactorTool::getFrontendActionFactory()::'lambda'(clang::ASTContext&)&, clang::ASTContext&>(std::__invoke_other, (anonymous namespace)::ClangRefactorTool::getFrontendActionFactory()::'lambda'(clang::ASTContext&)&, clang::ASTContext&) /usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/invoke.h:61:7
#15 0x0000562e7ed5e7ad std::enable_if<is_invocable_r_v<void, (anonymous namespace)::ClangRefactorTool::getFrontendActionFactory()::'lambda'(clang::ASTContext&)&, clang::ASTContext&>, void>::type std::__invoke_r<void, (anonymous namespace)::ClangRefactorTool::getFrontendActionFactory()::'lambda'(clang::ASTContext&)&, clang::ASTContext&>((anonymous namespace)::ClangRefactorTool::getFrontendActionFactory()::'lambda'(clang::ASTContext&)&, clang::ASTContext&) /usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/invoke.h:117:5
#16 0x0000562e7ed5e6f5 std::_Function_handler<void (clang::ASTContext&), (anonymous namespace)::ClangRefactorTool::getFrontendActionFactory()::'lambda'(clang::ASTContext&)>::_M_invoke(std::_Any_data const&, clang::ASTContext&) /usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/std_function.h:290:2
#17 0x0000562e7ed774c6 std::function<void (clang::ASTContext&)>::operator()(clang::ASTContext&) const /usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/std_function.h:591:2
#18 0x0000562e7ed5f3b1 (anonymous namespace)::ClangRefactorTool::getFrontendActionFactory()::ToolASTConsumer::HandleTranslationUnit(clang::ASTContext&) /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/tools/clang-refactor/ClangRefactor.cpp:441:7
#19 0x0000562e80984ffb clang::ParseAST(clang::Sema&, bool, bool) /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/lib/Parse/ParseAST.cpp:191:12
#20 0x0000562e800193d9 clang::ASTFrontendAction::ExecuteAction() /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/lib/Frontend/FrontendAction.cpp:1345:1
#21 0x0000562e80018e26 clang::FrontendAction::Execute() /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/lib/Frontend/FrontendAction.cpp:1229:7
#22 0x0000562e7ff2f0b3 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/lib/Frontend/CompilerInstance.cpp:1057:23
#23 0x0000562e8044f193 clang::tooling::FrontendActionFactory::runInvocation(std::shared_ptr<clang::CompilerInvocation>, clang::FileManager*, std::shared_ptr<clang::PCHContainerOperations>, clang::DiagnosticConsumer*) /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/lib/Tooling/Tooling.cpp:466:14
#24 0x0000562e8044f05b clang::tooling::ToolInvocation::runInvocation(char const*, clang::driver::Compilation*, std::shared_ptr<clang::CompilerInvocation>, std::shared_ptr<clang::PCHContainerOperations>) /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/lib/Tooling/Tooling.cpp:441:18
#25 0x0000562e8044e217 clang::tooling::ToolInvocation::run() /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/lib/Tooling/Tooling.cpp:426:3
#26 0x0000562e80450382 clang::tooling::ClangTool::run(clang::tooling::ToolAction*) /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/lib/Tooling/Tooling.cpp:624:11
#27 0x0000562e7ed595ea main /home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/clang/tools/clang-refactor/ClangRefactor.cpp:636:38
#28 0x00007f6311a433b8 (/lib/x86_64-linux-gnu/libc.so.6+0x2a3b8)
#29 0x00007f6311a4347b __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a47b)
#30 0x0000562e7ed591e5 _start (/home/nav/.distrobox/ubuntu-24-10-compiler-dev/llvm-project-dev/build/bin/clang-refactor+0x23741e5)
Aborted (core dumped)
```

</details>

(And similar invalid inputs like `<file>:<line>:-1` are caught in `ParsedSourceLocation::FromString`)

I could move the check up to the locations where `ParsedSourceLocation::FromString` and `ParsedSourceRange::FromString` are called, but I believe it should be possible to provide a proper front-end diagnostic without moving the check.
For context, this PR currently changes the behavior of the crash in #139375 to also omit the `diag::err_drv_invalid_value` diagnostic, but I could adjust it to hint at the right format.

What do you think is best here?

https://github.com/llvm/llvm-project/pull/139457