Give a warning when no dwo files are provided #94336

Labman-001 · 2024-06-04T10:55:07Z

In some scenarios based on the split-dwarf build process, the dwo file is not generated as expected(That is to say, no dwo file path is stored in the binary). When the llvm-dwp tool is called to generate the .dwp file, it will exit without any warning.

So, the plan is to prompt a warning to tell the user that the dwo file was not actually generated.

llvmbot · 2024-06-04T10:55:40Z

@llvm/pr-subscribers-debuginfo

Author: Jinjie Huang (Labman-001)

Changes

In some scenarios based on the split-dwarf build process, the dwo file is not generated as expected. When the llvm-dwp tool is called to generate the .dwp file, it will exit without any warning.
The plan is to prompt a warning to tell the user that the dwo files was not actually generated(That is to say, no dwo file path is stored in the binary).
<img width="173" alt="image" src="https://github.com/llvm/llvm-project/assets/150100070/c7e671fa-4558-4a41-8cc6-f894ab4356b8">

Full diff: https://github.com/llvm/llvm-project/pull/94336.diff

1 Files Affected:

(modified) llvm/tools/llvm-dwp/llvm-dwp.cpp (+3-1)

diff --git a/llvm/tools/llvm-dwp/llvm-dwp.cpp b/llvm/tools/llvm-dwp/llvm-dwp.cpp
index 81556b3ad4bcb..f0edaeb07f0ba 100644
--- a/llvm/tools/llvm-dwp/llvm-dwp.cpp
+++ b/llvm/tools/llvm-dwp/llvm-dwp.cpp
@@ -188,8 +188,10 @@ int llvm_dwp_main(int argc, char **argv, const llvm::ToolContext &) {
                         std::make_move_iterator(DWOs->end()));
   }
 
-  if (DWOFilenames.empty())
+  if (DWOFilenames.empty()) {
+    WithColor::defaultWarningHandler(make_error<DWPError>("No dwo files found!\n"));
     return 0;
+  }
 
   std::string ErrorStr;
   StringRef Context = "dwarf streamer init";

github-actions · 2024-06-04T10:58:56Z

✅ With the latest revision this PR passed the C/C++ code formatter.

Labman-001 · 2024-06-04T11:14:44Z

Can you help to review this? @dwblaikie @ayermolo thanks

ayermolo · 2024-06-04T14:17:21Z

Can you add a test?
Also maybe print out which file was not found?

dwblaikie · 2024-06-04T17:13:00Z

what does binutils dwp do in this case? (not that we should necessarily do the same thing)

(& generally warnings are all lower case, no ending punctuation (no ! or ., etc))

& yeah, test would be good

Labman-001 · 2024-06-07T03:23:30Z

Thank you for the review comments.

Also maybe print out which file was not found?

Since the scenario here targets the branch where the dwo path list is empty, there might not be a concept of "which" here. In regular scenarios, if a specific dwo file is missing, the 'write' function will report an error indicating that the corresponding file cannot be found.

what does binutils dwp do in this case? (not that we should necessarily do the same thing)

In my Debian 10 system(Binutils dwp 2.31.1), it will just trigger a core dump in this case, while llvm-dwp just quits silently：

> dwp -e xxx.out -o xxx.dwp
Segmentation fault (core dumped)

> llvm-dwp -e xxx.out -o xxx.dwp

So it may be helpful to give a warning here?

& yeah, test would be good

Yes, I will add a targeted test.

dwblaikie · 2024-06-07T18:14:43Z

I think the right explanation/error message would be "executable does not contain any skeleton units" (technically correct, but probably doesn't mean much to users) or "executable file does not contain any references to dwo files"?

"no dwo files found" sounds more like there weren't any dwo files on disk (and as a user I'd be asking "why didn't you tell me which dwo files you /tried/ to find/couldn't read")

Labman-001 · 2024-06-11T03:14:12Z

The message has been changed to "executable file does not contain any references to dwo files".
But In fact, there are two cases where DWOFilenames.empty() is true: one is when there are no references to dwo files in the skeleton, and the other is when no dwo files are given in the input.
Should we consider changing the message to "no dwo files are provided, or the executable file does not contain any references to dwo files"?

Labman-001 · 2024-06-14T10:16:53Z

please take a look @dwblaikie @ayermolo, thank you

ayermolo · 2024-06-14T19:07:57Z

llvm/test/tools/llvm-dwp/Inputs/empty_warning/non_split.s

@@ -0,0 +1,218 @@
+# Note: This file is compiled from the following code, for 


small nit. I think it's now ##

ayermolo

Can you simplify/reduce test. There is no need to rely on printf I think.

…:Labman-001/llvm-project into users/huangjinjie/llvm-dwp-empty-warning

Labman-001 · 2024-06-17T03:53:21Z

Sorry, I might not have fully understood what is meant by "simplify/reduce test" here. Does it mean that I should consider not printing and checking the warning information? I'm not quite sure if it's okay to check without it

ayermolo · 2024-06-17T17:18:14Z

The test itself:

include <stdio.h>
int main() {
    printf("hello\n");
    return 0;
}

Can you change it not to rely on stdio.h. Just returning 0 should be enough to repro correct?

dwblaikie · 2024-06-18T17:46:50Z

Would https://llvm.org/docs/TestingGuide.html#elaborated-tests help make the tests cleaner/easier to maintain? (or is there enough post-processing of the files, needs other tools, etc?)

Labman-001 · 2024-06-19T13:14:30Z

Thank you for the suggestion. I have changed it to compile a non-split-dwarf binary from the source code only, so that it does not contain any skeleton information. Originally, I introduced the .s file to modify the skeleton in the split-dwarf binary to trigger an error when the corresponding .dwo file could not be found (but this does not seem to be my target scenario?), so I have removed it.

…injie/llvm-dwp-empty-warning

ayermolo · 2024-06-19T15:03:20Z

LGTM. Thanks for your patience.

Labman-001 · 2024-06-24T03:33:51Z

@dwblaikie Could you please help me review the test? Thank you.

dwblaikie · 2024-06-24T17:24:19Z

llvm/test/tools/llvm-dwp/X86/empty_warning.test

@@ -0,0 +1,9 @@
+; RUN: rm -rf %t && mkdir -p %t && split-file %s %t && cd %t
+; RUN: clang -g -gdwarf-4 %t/non_split.c -o %t/non_split.o


We can't run clang as part of llvm's tests (it's a layering violation - clang depends on llvm, not the other way around).

This test should contain something more basic - perhaps an empty or trivial assembly file that llvm-mc -g could be used to compile to an object file and that might be enough for the situation?

oh didn't know that.
Why did the test pass? Thought there would be an error "clang not found" or something like that.

lit RUN lines basically boil down to a shell script, so it inherits the system PATH - but if you ran the tests on a machine that didn't have clang installed (and didn't build clang) the test would fail, so far as I understand.

Labman-001 · 2024-06-26T09:20:15Z

Thank you, I have changed it to use llvm-mc to generate the binary for testing.

dwblaikie

This is OK
Though, if you can - https://llvm.org/docs/TestingGuide.html#elaborated-tests - use this so you can include the source code in the test, and make it easy to regenerate, if needed?

Labman-001 · 2024-06-27T06:39:31Z

Yes, the tool is useful. And source code has been included in the test.

In some scenarios based on the split-dwarf build process, the dwo file is not generated as expected(That is to say, no dwo file path is stored in the binary). When the llvm-dwp tool is called to generate the .dwp file, it will exit without any warning. So, the plan is to prompt a warning to tell the user that the dwo file was not actually generated. <img width="699" alt="image" src="https://github.com/llvm/llvm-project/assets/150100070/5e5742f6-daad-450f-87e9-cb25449c3c7a">

Give a warning when now dwo found

6737787

llvmbot added the debuginfo label Jun 4, 2024

Labman-001 added 2 commits June 4, 2024 19:00

Give a warning when no dwo found

5899de9

Give a warning when no dwo found

2a54154

Labman-001 added 2 commits June 7, 2024 17:21

add test

cca7580

update test

4ee62b0

Labman-001 added 4 commits June 9, 2024 22:20

modify warnging messages

7cea95d

modify warnging messages

9ad6ffa

modify warnging messages

4b42331

modify warnging messages

18c730d

Labman-001 changed the title ~~Give a warning when no dwo files are found~~ Give a warning when no dwo files are provided Jun 11, 2024

ayermolo reviewed Jun 14, 2024

View reviewed changes

ayermolo approved these changes Jun 14, 2024

View reviewed changes

ayermolo requested changes Jun 14, 2024

View reviewed changes

Labman-001 added 4 commits June 17, 2024 10:57

modify test

42d92fc

Merge branch 'users/huangjinjie/llvm-dwp-empty-warning' of github.com…

e3f75e2

…:Labman-001/llvm-project into users/huangjinjie/llvm-dwp-empty-warning

modify test

bd2a43f

modify test

853f2a7

modify test rm printf

f1eae5f

modify test rm printf

d95fa91

Labman-001 added 2 commits June 19, 2024 20:42

use split-file

058e77d

use split-file

641c166

Labman-001 added 2 commits June 19, 2024 22:51

Merge branch 'main' of github.com:llvm/llvm-project into users/huangj…

39c6ca1

…injie/llvm-dwp-empty-warning

modify test

495f64d

ayermolo approved these changes Jun 19, 2024

View reviewed changes

dwblaikie reviewed Jun 24, 2024

View reviewed changes

Labman-001 added 2 commits June 26, 2024 15:02

use llvm-mc to gen

1056209

modify test

0a387c2

dwblaikie approved these changes Jun 26, 2024

View reviewed changes

Labman-001 added 2 commits June 27, 2024 10:57

modify test

13494f5

modify test to gen .s

a735350

dwblaikie merged commit d7cd41e into llvm:main Jun 27, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Give a warning when no dwo files are provided #94336

Give a warning when no dwo files are provided #94336

Labman-001 commented Jun 4, 2024 •

edited

Loading

llvmbot commented Jun 4, 2024

github-actions bot commented Jun 4, 2024 •

edited

Loading

Labman-001 commented Jun 4, 2024 •

edited

Loading

ayermolo commented Jun 4, 2024

dwblaikie commented Jun 4, 2024

Labman-001 commented Jun 7, 2024 •

edited

Loading

dwblaikie commented Jun 7, 2024

Labman-001 commented Jun 11, 2024 •

edited

Loading

Labman-001 commented Jun 14, 2024

ayermolo Jun 14, 2024

ayermolo left a comment

Labman-001 commented Jun 17, 2024 •

edited

Loading

ayermolo commented Jun 17, 2024 •

edited

Loading

dwblaikie commented Jun 18, 2024

Labman-001 commented Jun 19, 2024 •

edited

Loading

ayermolo commented Jun 19, 2024

Labman-001 commented Jun 24, 2024

dwblaikie Jun 24, 2024

ayermolo Jun 24, 2024

dwblaikie Jun 24, 2024

Labman-001 commented Jun 26, 2024

dwblaikie left a comment

Labman-001 commented Jun 27, 2024

		@@ -0,0 +1,218 @@
		# Note: This file is compiled from the following code, for

		@@ -0,0 +1,9 @@
		; RUN: rm -rf %t && mkdir -p %t && split-file %s %t && cd %t
		; RUN: clang -g -gdwarf-4 %t/non_split.c -o %t/non_split.o

Give a warning when no dwo files are provided #94336

Give a warning when no dwo files are provided #94336

Conversation

Labman-001 commented Jun 4, 2024 • edited Loading

llvmbot commented Jun 4, 2024

github-actions bot commented Jun 4, 2024 • edited Loading

Labman-001 commented Jun 4, 2024 • edited Loading

ayermolo commented Jun 4, 2024

dwblaikie commented Jun 4, 2024

Labman-001 commented Jun 7, 2024 • edited Loading

dwblaikie commented Jun 7, 2024

Labman-001 commented Jun 11, 2024 • edited Loading

Labman-001 commented Jun 14, 2024

ayermolo Jun 14, 2024

Choose a reason for hiding this comment

ayermolo left a comment

Choose a reason for hiding this comment

Labman-001 commented Jun 17, 2024 • edited Loading

ayermolo commented Jun 17, 2024 • edited Loading

dwblaikie commented Jun 18, 2024

Labman-001 commented Jun 19, 2024 • edited Loading

ayermolo commented Jun 19, 2024

Labman-001 commented Jun 24, 2024

dwblaikie Jun 24, 2024

Choose a reason for hiding this comment

ayermolo Jun 24, 2024

Choose a reason for hiding this comment

dwblaikie Jun 24, 2024

Choose a reason for hiding this comment

Labman-001 commented Jun 26, 2024

dwblaikie left a comment

Choose a reason for hiding this comment

Labman-001 commented Jun 27, 2024

Labman-001 commented Jun 4, 2024 •

edited

Loading

github-actions bot commented Jun 4, 2024 •

edited

Loading

Labman-001 commented Jun 4, 2024 •

edited

Loading

Labman-001 commented Jun 7, 2024 •

edited

Loading

Labman-001 commented Jun 11, 2024 •

edited

Loading

Labman-001 commented Jun 17, 2024 •

edited

Loading

ayermolo commented Jun 17, 2024 •

edited

Loading

Labman-001 commented Jun 19, 2024 •

edited

Loading