Commit 35039eb6b199 ("x86: Show symbol name if insn decoder test failed") included symbol lines in the post-processed objdump output consumed by the insn decoder test. This broke the `instuction lines == total lines` property that `insn_decoder_test.c` relied upon to print the offending line's number in error messages. This has the consequence that the line number reported on a test failure is unreated to, and much smaller than, the line that actually caused the problem.
Add a new variable that counts the combined (insn+symbol) line count and report this in the error message.
Fixes: 35039eb6b199 ("x86: Show symbol name if insn decoder test failed") Cc: stable@vger.kernel.org Reviewed-by: Miguel Ojeda ojeda@kernel.org Tested-by: Miguel Ojeda ojeda@kernel.org Reported-by: John Baublitz john.m.baublitz@gmail.com Debugged-by: John Baublitz john.m.baublitz@gmail.com Signed-off-by: Valentin Obst kernel@valentinobst.de --- See v2's commit message and [1] for context why this bug made debugging a test failure harder than necessary.
[1]: https://rust-for-linux.zulipchat.com/#narrow/stream/291565-Help/topic/insn_d...
Changes in v3: - Add Cc stable tag in sign-off area. - Make commit message less verbose. - Link to v2: https://lore.kernel.org/r/20240223-x86-insn-decoder-line-fix-v2-1-cde49c69f4...
Changes in v2: - Added tags 'Reviewed-by', 'Tested-by', 'Reported-by', 'Debugged-by', 'Link', and 'Fixes'. - Explain why this patch fixes the commit mentioned in the 'Fixes' tag. - CCed the stable list and sent to all x86 maintainers. - Link to v1: https://lore.kernel.org/r/20240221-x86-insn-decoder-line-fix-v1-1-47cd5a1718... --- arch/x86/tools/insn_decoder_test.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/x86/tools/insn_decoder_test.c b/arch/x86/tools/insn_decoder_test.c index 472540aeabc2..727017a3c3c7 100644 --- a/arch/x86/tools/insn_decoder_test.c +++ b/arch/x86/tools/insn_decoder_test.c @@ -114,6 +114,7 @@ int main(int argc, char **argv) unsigned char insn_buff[16]; struct insn insn; int insns = 0; + int lines = 0; int warnings = 0;
parse_args(argc, argv); @@ -123,6 +124,8 @@ int main(int argc, char **argv) int nb = 0, ret; unsigned int b;
+ lines++; + if (line[0] == '<') { /* Symbol line */ strcpy(sym, line); @@ -134,12 +137,12 @@ int main(int argc, char **argv) strcpy(copy, line); tab1 = strchr(copy, '\t'); if (!tab1) - malformed_line(line, insns); + malformed_line(line, lines); s = tab1 + 1; s += strspn(s, " "); tab2 = strchr(s, '\t'); if (!tab2) - malformed_line(line, insns); + malformed_line(line, lines); *tab2 = '\0'; /* Characters beyond tab2 aren't examined */ while (s < tab2) { if (sscanf(s, "%x", &b) == 1) {
--- base-commit: 4cece764965020c22cff7665b18a012006359095 change-id: 20240221-x86-insn-decoder-line-fix-7b1f2e1732ff
Best regards, -- Valentin Obst kernel@valentinobst.de
On Fri, 29 Mar 2024 12:31:58 +0100 Valentin Obst kernel@valentinobst.de wrote:
Commit 35039eb6b199 ("x86: Show symbol name if insn decoder test failed") included symbol lines in the post-processed objdump output consumed by the insn decoder test. This broke the `instuction lines == total lines` property that `insn_decoder_test.c` relied upon to print the offending line's number in error messages. This has the consequence that the line number reported on a test failure is unreated to, and much smaller than, the line that actually caused the problem.
Add a new variable that counts the combined (insn+symbol) line count and report this in the error message.
This looks good to me. Thanks!
Acked-by: Masami Hiramatsu (Google) mhiramat@kernel.org
Fixes: 35039eb6b199 ("x86: Show symbol name if insn decoder test failed") Cc: stable@vger.kernel.org Reviewed-by: Miguel Ojeda ojeda@kernel.org Tested-by: Miguel Ojeda ojeda@kernel.org Reported-by: John Baublitz john.m.baublitz@gmail.com Debugged-by: John Baublitz john.m.baublitz@gmail.com Signed-off-by: Valentin Obst kernel@valentinobst.de
See v2's commit message and [1] for context why this bug made debugging a test failure harder than necessary.
Changes in v3:
- Add Cc stable tag in sign-off area.
- Make commit message less verbose.
- Link to v2: https://lore.kernel.org/r/20240223-x86-insn-decoder-line-fix-v2-1-cde49c69f4...
Changes in v2:
- Added tags 'Reviewed-by', 'Tested-by', 'Reported-by', 'Debugged-by', 'Link', and 'Fixes'.
- Explain why this patch fixes the commit mentioned in the 'Fixes' tag.
- CCed the stable list and sent to all x86 maintainers.
- Link to v1: https://lore.kernel.org/r/20240221-x86-insn-decoder-line-fix-v1-1-47cd5a1718...
arch/x86/tools/insn_decoder_test.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/x86/tools/insn_decoder_test.c b/arch/x86/tools/insn_decoder_test.c index 472540aeabc2..727017a3c3c7 100644 --- a/arch/x86/tools/insn_decoder_test.c +++ b/arch/x86/tools/insn_decoder_test.c @@ -114,6 +114,7 @@ int main(int argc, char **argv) unsigned char insn_buff[16]; struct insn insn; int insns = 0;
int lines = 0; int warnings = 0;
parse_args(argc, argv);
@@ -123,6 +124,8 @@ int main(int argc, char **argv) int nb = 0, ret; unsigned int b;
lines++;
- if (line[0] == '<') { /* Symbol line */ strcpy(sym, line);
@@ -134,12 +137,12 @@ int main(int argc, char **argv) strcpy(copy, line); tab1 = strchr(copy, '\t'); if (!tab1)
malformed_line(line, insns);
s = tab1 + 1; s += strspn(s, " "); tab2 = strchr(s, '\t'); if (!tab2)malformed_line(line, lines);
malformed_line(line, insns);
*tab2 = '\0'; /* Characters beyond tab2 aren't examined */ while (s < tab2) { if (sscanf(s, "%x", &b) == 1) {malformed_line(line, lines);
base-commit: 4cece764965020c22cff7665b18a012006359095 change-id: 20240221-x86-insn-decoder-line-fix-7b1f2e1732ff
Best regards,
Valentin Obst kernel@valentinobst.de
Commit 35039eb6b199 ("x86: Show symbol name if insn decoder test failed") included symbol lines in the post-processed objdump output consumed by the insn decoder test. This broke the `instuction lines == total lines` property that `insn_decoder_test.c` relied upon to print the offending line's number in error messages. This has the consequence that the line number reported on a test failure is unreated to, and much smaller than, the line that actually caused the problem.
Add a new variable that counts the combined (insn+symbol) line count and report this in the error message.
This looks good to me. Thanks!
Acked-by: Masami Hiramatsu (Google) mhiramat@kernel.org
Thanks!
So what is the status on this one? Is there someone here who can take it through their tree s.t. it might make it into 6.10-rc1 or 6.11-rc1?
- Best Valentin
Fixes: 35039eb6b199 ("x86: Show symbol name if insn decoder test failed") Cc: stable@vger.kernel.org Reviewed-by: Miguel Ojeda ojeda@kernel.org Tested-by: Miguel Ojeda ojeda@kernel.org Reported-by: John Baublitz john.m.baublitz@gmail.com Debugged-by: John Baublitz john.m.baublitz@gmail.com Signed-off-by: Valentin Obst kernel@valentinobst.de
-- Masami Hiramatsu (Google) mhiramat@kernel.org
linux-stable-mirror@lists.linaro.org