Running the test "Miscellaneous Intel PT testing" with a build with
-fsanitize=memory and -fsanitize-memory-track-origins I saw:
```
==1257749==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x5581c00a76b3 in intel_pt_sample_flags tools/perf/util/intel-pt.c:1527:17
#1 0x5581c00c5cf6 in intel_pt_run_decoder tools/perf/util/intel-pt.c:2961:3
#2 0x5581c00968f8 in intel_pt_process_timeless_queues tools/perf/util/intel-pt.c:3074:4
#3 0x5581c007cf49 in intel_pt_process_event tools/perf/util/intel-pt.c:3482:10
#4 0x5581bffa269a in auxtrace__process_event tools/perf/util/auxtrace.c:2830:9
#5 0x5581bfb826c0 in perf_session__deliver_event tools/perf/util/session.c:1649:8
#6 0x5581bfba1d7f in perf_session__process_event tools/perf/util/session.c:1891:9
#7 0x5581bfba82e4 in process_simple tools/perf/util/session.c:2452:9
#8 0x5581bfbabcc3 in reader__read_event tools/perf/util/session.c:2381:14
#9 0x5581bfbad942 in reader__process_events tools/perf/util/session.c:2430:8
#10 0x5581bfb78256 in __perf_session__process_events tools/perf/util/session.c:2477:8
#11 0x5581bfb702c4 in perf_session__process_events tools/perf/util/session.c:2643:9
#12 0x5581bf2da266 in __cmd_script tools/perf/builtin-script.c:2855:8
#13 0x5581bf2bfcdd in cmd_script tools/perf/builtin-script.c:4402:8
#14 0x5581bf67004b in run_builtin tools/perf/perf.c:350:11
#15 0x5581bf66b8ea in handle_internal_command tools/perf/perf.c:403:8
#16 0x5581bf66f527 in run_argv tools/perf/perf.c:447:2
#17 0x5581bf669d2d in main tools/perf/perf.c:561:3
Uninitialized value was stored to memory at
#0 0x5581c005ddf8 in intel_pt_walk_insn tools/perf/util/intel-pt-decoder/intel-pt-decoder.c:1256:25
#1 0x5581c001a932 in intel_pt_walk_fup tools/perf/util/intel-pt-decoder/intel-pt-decoder.c:1428:9
#2 0x5581c000f76c in intel_pt_walk_trace tools/perf/util/intel-pt-decoder/intel-pt-decoder.c:3299:10
#3 0x5581c000899b in intel_pt_decode tools/perf/util/intel-pt-decoder/intel-pt-decoder.c:3988:10
#4 0x5581c00c5180 in intel_pt_run_decoder tools/perf/util/intel-pt.c:2941:11
#5 0x5581c00968f8 in intel_pt_process_timeless_queues tools/perf/util/intel-pt.c:3074:4
#6 0x5581c007cf49 in intel_pt_process_event tools/perf/util/intel-pt.c:3482:10
#7 0x5581bffa269a in auxtrace__process_event tools/perf/util/auxtrace.c:2830:9
#8 0x5581bfb826c0 in perf_session__deliver_event tools/perf/util/session.c:1649:8
#9 0x5581bfba1d7f in perf_session__process_event tools/perf/util/session.c:1891:9
#10 0x5581bfba82e4 in process_simple tools/perf/util/session.c:2452:9
#11 0x5581bfbabcc3 in reader__read_event tools/perf/util/session.c:2381:14
#12 0x5581bfbad942 in reader__process_events tools/perf/util/session.c:2430:8
#13 0x5581bfb78256 in __perf_session__process_events tools/perf/util/session.c:2477:8
#14 0x5581bfb702c4 in perf_session__process_events tools/perf/util/session.c:2643:9
#15 0x5581bf2da266 in __cmd_script tools/perf/builtin-script.c:2855:8
#16 0x5581bf2bfcdd in cmd_script tools/perf/builtin-script.c:4402:8
#17 0x5581bf67004b in run_builtin tools/perf/perf.c:350:11
#18 0x5581bf66b8ea in handle_internal_command tools/perf/perf.c:403:8
#19 0x5581bf66f527 in run_argv tools/perf/perf.c:447:2
```
Adding a curly brace initializer for the intel_pt_insn in
intel_pt_walk_fup rectifies this, however, there may be other issues
lurking behind this so initialize all similar instances.
Fixes: f4aa081949e7 ("perf tools: Add Intel PT decoder")
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/intel-pt-decoder/intel-pt-decoder.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
index b450178e3420..b4a95af2e4cc 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
@@ -1115,7 +1115,7 @@ static void intel_pt_sample_insn(struct intel_pt_decoder *decoder)
*/
static void intel_pt_sample_fup_insn(struct intel_pt_decoder *decoder)
{
- struct intel_pt_insn intel_pt_insn;
+ struct intel_pt_insn intel_pt_insn = {};
uint64_t max_insn_cnt, insn_cnt = 0;
int err;
@@ -1418,7 +1418,7 @@ static inline bool intel_pt_fup_with_nlip(struct intel_pt_decoder *decoder,
static int intel_pt_walk_fup(struct intel_pt_decoder *decoder)
{
- struct intel_pt_insn intel_pt_insn;
+ struct intel_pt_insn intel_pt_insn = {};
uint64_t ip;
int err;
@@ -1461,7 +1461,7 @@ static int intel_pt_walk_fup(struct intel_pt_decoder *decoder)
static int intel_pt_walk_tip(struct intel_pt_decoder *decoder)
{
- struct intel_pt_insn intel_pt_insn;
+ struct intel_pt_insn intel_pt_insn = {};
int err;
err = intel_pt_walk_insn(decoder, &intel_pt_insn, 0);
@@ -1626,7 +1626,7 @@ static int intel_pt_emulated_ptwrite(struct intel_pt_decoder *decoder)
static int intel_pt_walk_tnt(struct intel_pt_decoder *decoder)
{
- struct intel_pt_insn intel_pt_insn;
+ struct intel_pt_insn intel_pt_insn = {};
int err;
while (1) {
--
2.44.0.291.gc1ea87d7ee-goog
On 25/03/24 17:44, Ian Rogers wrote:
> On Wed, Mar 20, 2024 at 9:26 AM Ian Rogers <[email protected]> wrote:
>>
>> Running the test "Miscellaneous Intel PT testing" with a build with
>> -fsanitize=memory and -fsanitize-memory-track-origins I saw:
>>
>> ```
>> ==1257749==WARNING: MemorySanitizer: use-of-uninitialized-value
>> #0 0x5581c00a76b3 in intel_pt_sample_flags tools/perf/util/intel-pt.c:1527:17
>> #1 0x5581c00c5cf6 in intel_pt_run_decoder tools/perf/util/intel-pt.c:2961:3
>> #2 0x5581c00968f8 in intel_pt_process_timeless_queues tools/perf/util/intel-pt.c:3074:4
>> #3 0x5581c007cf49 in intel_pt_process_event tools/perf/util/intel-pt.c:3482:10
>> #4 0x5581bffa269a in auxtrace__process_event tools/perf/util/auxtrace.c:2830:9
>> #5 0x5581bfb826c0 in perf_session__deliver_event tools/perf/util/session.c:1649:8
>> #6 0x5581bfba1d7f in perf_session__process_event tools/perf/util/session.c:1891:9
>> #7 0x5581bfba82e4 in process_simple tools/perf/util/session.c:2452:9
>> #8 0x5581bfbabcc3 in reader__read_event tools/perf/util/session.c:2381:14
>> #9 0x5581bfbad942 in reader__process_events tools/perf/util/session.c:2430:8
>> #10 0x5581bfb78256 in __perf_session__process_events tools/perf/util/session.c:2477:8
>> #11 0x5581bfb702c4 in perf_session__process_events tools/perf/util/session.c:2643:9
>> #12 0x5581bf2da266 in __cmd_script tools/perf/builtin-script.c:2855:8
>> #13 0x5581bf2bfcdd in cmd_script tools/perf/builtin-script.c:4402:8
>> #14 0x5581bf67004b in run_builtin tools/perf/perf.c:350:11
>> #15 0x5581bf66b8ea in handle_internal_command tools/perf/perf.c:403:8
>> #16 0x5581bf66f527 in run_argv tools/perf/perf.c:447:2
>> #17 0x5581bf669d2d in main tools/perf/perf.c:561:3
>>
>> Uninitialized value was stored to memory at
>> #0 0x5581c005ddf8 in intel_pt_walk_insn tools/perf/util/intel-pt-decoder/intel-pt-decoder.c:1256:25
>> #1 0x5581c001a932 in intel_pt_walk_fup tools/perf/util/intel-pt-decoder/intel-pt-decoder.c:1428:9
>> #2 0x5581c000f76c in intel_pt_walk_trace tools/perf/util/intel-pt-decoder/intel-pt-decoder.c:3299:10
>> #3 0x5581c000899b in intel_pt_decode tools/perf/util/intel-pt-decoder/intel-pt-decoder.c:3988:10
>> #4 0x5581c00c5180 in intel_pt_run_decoder tools/perf/util/intel-pt.c:2941:11
>> #5 0x5581c00968f8 in intel_pt_process_timeless_queues tools/perf/util/intel-pt.c:3074:4
>> #6 0x5581c007cf49 in intel_pt_process_event tools/perf/util/intel-pt.c:3482:10
>> #7 0x5581bffa269a in auxtrace__process_event tools/perf/util/auxtrace.c:2830:9
>> #8 0x5581bfb826c0 in perf_session__deliver_event tools/perf/util/session.c:1649:8
>> #9 0x5581bfba1d7f in perf_session__process_event tools/perf/util/session.c:1891:9
>> #10 0x5581bfba82e4 in process_simple tools/perf/util/session.c:2452:9
>> #11 0x5581bfbabcc3 in reader__read_event tools/perf/util/session.c:2381:14
>> #12 0x5581bfbad942 in reader__process_events tools/perf/util/session.c:2430:8
>> #13 0x5581bfb78256 in __perf_session__process_events tools/perf/util/session.c:2477:8
>> #14 0x5581bfb702c4 in perf_session__process_events tools/perf/util/session.c:2643:9
>> #15 0x5581bf2da266 in __cmd_script tools/perf/builtin-script.c:2855:8
>> #16 0x5581bf2bfcdd in cmd_script tools/perf/builtin-script.c:4402:8
>> #17 0x5581bf67004b in run_builtin tools/perf/perf.c:350:11
>> #18 0x5581bf66b8ea in handle_internal_command tools/perf/perf.c:403:8
>> #19 0x5581bf66f527 in run_argv tools/perf/perf.c:447:2
>> ```
>>
>> Adding a curly brace initializer for the intel_pt_insn in
>> intel_pt_walk_fup rectifies this, however, there may be other issues
>> lurking behind this so initialize all similar instances.
>>
>> Fixes: f4aa081949e7 ("perf tools: Add Intel PT decoder")
>> Signed-off-by: Ian Rogers <[email protected]>
>
> Adrian, could you take a look at this for the sake of tests passing with msan.
I did have a look, but I found msan so slow and full of errors
that it never gave any results.
However, it is easy enough to instrument some debugging which is
what I am still looking at.
Just initializing intel_pt_insn is not quite right, so I need
to check things a bit more.
>
> Thanks,
> Ian
>
>> ---
>> tools/perf/util/intel-pt-decoder/intel-pt-decoder.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
>> index b450178e3420..b4a95af2e4cc 100644
>> --- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
>> +++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
>> @@ -1115,7 +1115,7 @@ static void intel_pt_sample_insn(struct intel_pt_decoder *decoder)
>> */
>> static void intel_pt_sample_fup_insn(struct intel_pt_decoder *decoder)
>> {
>> - struct intel_pt_insn intel_pt_insn;
>> + struct intel_pt_insn intel_pt_insn = {};
>> uint64_t max_insn_cnt, insn_cnt = 0;
>> int err;
>>
>> @@ -1418,7 +1418,7 @@ static inline bool intel_pt_fup_with_nlip(struct intel_pt_decoder *decoder,
>>
>> static int intel_pt_walk_fup(struct intel_pt_decoder *decoder)
>> {
>> - struct intel_pt_insn intel_pt_insn;
>> + struct intel_pt_insn intel_pt_insn = {};
>> uint64_t ip;
>> int err;
>>
>> @@ -1461,7 +1461,7 @@ static int intel_pt_walk_fup(struct intel_pt_decoder *decoder)
>>
>> static int intel_pt_walk_tip(struct intel_pt_decoder *decoder)
>> {
>> - struct intel_pt_insn intel_pt_insn;
>> + struct intel_pt_insn intel_pt_insn = {};
>> int err;
>>
>> err = intel_pt_walk_insn(decoder, &intel_pt_insn, 0);
>> @@ -1626,7 +1626,7 @@ static int intel_pt_emulated_ptwrite(struct intel_pt_decoder *decoder)
>>
>> static int intel_pt_walk_tnt(struct intel_pt_decoder *decoder)
>> {
>> - struct intel_pt_insn intel_pt_insn;
>> + struct intel_pt_insn intel_pt_insn = {};
>> int err;
>>
>> while (1) {
>> --
>> 2.44.0.291.gc1ea87d7ee-goog
>>
MemorySanitizer discovered instances where the instruction op value was
not assigned.:
WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x5581c00a76b3 in intel_pt_sample_flags tools/perf/util/intel-pt.c:1527:17
Uninitialized value was stored to memory at
#0 0x5581c005ddf8 in intel_pt_walk_insn tools/perf/util/intel-pt-decoder/intel-pt-decoder.c:1256:25
The op value is used to set branch flags for branch instructions
encountered when walking the code, so fix by setting op to
INTEL_PT_OP_OTHER in other cases.
Reported-by: Ian Rogers <[email protected]>
Closes: https://lore.kernel.org/linux-perf-users/[email protected]/
Signed-off-by: Adrian Hunter <[email protected]>
---
tools/perf/util/intel-pt-decoder/intel-pt-decoder.c | 2 ++
tools/perf/util/intel-pt.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
index b450178e3420..e733f6b1f7ac 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
@@ -1319,6 +1319,8 @@ static bool intel_pt_fup_event(struct intel_pt_decoder *decoder, bool no_tip)
bool ret = false;
decoder->state.type &= ~INTEL_PT_BRANCH;
+ decoder->state.insn_op = INTEL_PT_OP_OTHER;
+ decoder->state.insn_len = 0;
if (decoder->set_fup_cfe_ip || decoder->set_fup_cfe) {
bool ip = decoder->set_fup_cfe_ip;
diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
index f38893e0b036..4db9a098f592 100644
--- a/tools/perf/util/intel-pt.c
+++ b/tools/perf/util/intel-pt.c
@@ -764,6 +764,7 @@ static int intel_pt_walk_next_insn(struct intel_pt_insn *intel_pt_insn,
addr_location__init(&al);
intel_pt_insn->length = 0;
+ intel_pt_insn->op = INTEL_PT_OP_OTHER;
if (to_ip && *ip == to_ip)
goto out_no_cache;
@@ -898,6 +899,7 @@ static int intel_pt_walk_next_insn(struct intel_pt_insn *intel_pt_insn,
if (to_ip && *ip == to_ip) {
intel_pt_insn->length = 0;
+ intel_pt_insn->op = INTEL_PT_OP_OTHER;
goto out_no_cache;
}
--
2.34.1
On Tue, Mar 26, 2024 at 1:32 AM Adrian Hunter <[email protected]> wrote:
>
> MemorySanitizer discovered instances where the instruction op value was
> not assigned.:
>
> WARNING: MemorySanitizer: use-of-uninitialized-value
> #0 0x5581c00a76b3 in intel_pt_sample_flags tools/perf/util/intel-pt.c:1527:17
> Uninitialized value was stored to memory at
> #0 0x5581c005ddf8 in intel_pt_walk_insn tools/perf/util/intel-pt-decoder/intel-pt-decoder.c:1256:25
>
> The op value is used to set branch flags for branch instructions
> encountered when walking the code, so fix by setting op to
> INTEL_PT_OP_OTHER in other cases.
>
> Reported-by: Ian Rogers <[email protected]>
> Closes: https://lore.kernel.org/linux-perf-users/[email protected]/
> Signed-off-by: Adrian Hunter <[email protected]>
Great, thanks! Should it have a Fixes tag like:
Fixes: 4c761d805bb2 ("perf intel-pt: Fix intel_pt_fup_event()
assumptions about setting state type")
Tested-by: Ian Rogers <[email protected]>
Ian
> ---
> tools/perf/util/intel-pt-decoder/intel-pt-decoder.c | 2 ++
> tools/perf/util/intel-pt.c | 2 ++
> 2 files changed, 4 insertions(+)
>
> diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
> index b450178e3420..e733f6b1f7ac 100644
> --- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
> +++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
> @@ -1319,6 +1319,8 @@ static bool intel_pt_fup_event(struct intel_pt_decoder *decoder, bool no_tip)
> bool ret = false;
>
> decoder->state.type &= ~INTEL_PT_BRANCH;
> + decoder->state.insn_op = INTEL_PT_OP_OTHER;
> + decoder->state.insn_len = 0;
>
> if (decoder->set_fup_cfe_ip || decoder->set_fup_cfe) {
> bool ip = decoder->set_fup_cfe_ip;
> diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
> index f38893e0b036..4db9a098f592 100644
> --- a/tools/perf/util/intel-pt.c
> +++ b/tools/perf/util/intel-pt.c
> @@ -764,6 +764,7 @@ static int intel_pt_walk_next_insn(struct intel_pt_insn *intel_pt_insn,
>
> addr_location__init(&al);
> intel_pt_insn->length = 0;
> + intel_pt_insn->op = INTEL_PT_OP_OTHER;
>
> if (to_ip && *ip == to_ip)
> goto out_no_cache;
> @@ -898,6 +899,7 @@ static int intel_pt_walk_next_insn(struct intel_pt_insn *intel_pt_insn,
>
> if (to_ip && *ip == to_ip) {
> intel_pt_insn->length = 0;
> + intel_pt_insn->op = INTEL_PT_OP_OTHER;
> goto out_no_cache;
> }
>
> --
> 2.34.1
>
On 26/03/24 18:13, Ian Rogers wrote:
> On Tue, Mar 26, 2024 at 1:32 AM Adrian Hunter <[email protected]> wrote:
>>
>> MemorySanitizer discovered instances where the instruction op value was
>> not assigned.:
>>
>> WARNING: MemorySanitizer: use-of-uninitialized-value
>> #0 0x5581c00a76b3 in intel_pt_sample_flags tools/perf/util/intel-pt.c:1527:17
>> Uninitialized value was stored to memory at
>> #0 0x5581c005ddf8 in intel_pt_walk_insn tools/perf/util/intel-pt-decoder/intel-pt-decoder.c:1256:25
>>
>> The op value is used to set branch flags for branch instructions
>> encountered when walking the code, so fix by setting op to
>> INTEL_PT_OP_OTHER in other cases.
>>
>> Reported-by: Ian Rogers <[email protected]>
>> Closes: https://lore.kernel.org/linux-perf-users/[email protected]/
>> Signed-off-by: Adrian Hunter <[email protected]>
>
> Great, thanks! Should it have a Fixes tag like:
>
> Fixes: 4c761d805bb2 ("perf intel-pt: Fix intel_pt_fup_event()
> assumptions about setting state type")
Yes, I should have put a fixes tag, cc stable. I think the issue
has always been there, so:
Fixes: 90e457f7be087 ("perf tools: Add Intel PT support")
Cc: [email protected]
>
> Tested-by: Ian Rogers <[email protected]>
>
> Ian
>
>> ---
>> tools/perf/util/intel-pt-decoder/intel-pt-decoder.c | 2 ++
>> tools/perf/util/intel-pt.c | 2 ++
>> 2 files changed, 4 insertions(+)
>>
>> diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
>> index b450178e3420..e733f6b1f7ac 100644
>> --- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
>> +++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
>> @@ -1319,6 +1319,8 @@ static bool intel_pt_fup_event(struct intel_pt_decoder *decoder, bool no_tip)
>> bool ret = false;
>>
>> decoder->state.type &= ~INTEL_PT_BRANCH;
>> + decoder->state.insn_op = INTEL_PT_OP_OTHER;
>> + decoder->state.insn_len = 0;
>>
>> if (decoder->set_fup_cfe_ip || decoder->set_fup_cfe) {
>> bool ip = decoder->set_fup_cfe_ip;
>> diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
>> index f38893e0b036..4db9a098f592 100644
>> --- a/tools/perf/util/intel-pt.c
>> +++ b/tools/perf/util/intel-pt.c
>> @@ -764,6 +764,7 @@ static int intel_pt_walk_next_insn(struct intel_pt_insn *intel_pt_insn,
>>
>> addr_location__init(&al);
>> intel_pt_insn->length = 0;
>> + intel_pt_insn->op = INTEL_PT_OP_OTHER;
>>
>> if (to_ip && *ip == to_ip)
>> goto out_no_cache;
>> @@ -898,6 +899,7 @@ static int intel_pt_walk_next_insn(struct intel_pt_insn *intel_pt_insn,
>>
>> if (to_ip && *ip == to_ip) {
>> intel_pt_insn->length = 0;
>> + intel_pt_insn->op = INTEL_PT_OP_OTHER;
>> goto out_no_cache;
>> }
>>
>> --
>> 2.34.1
>>
On Wed, Mar 20, 2024 at 9:26 AM Ian Rogers <[email protected]> wrote:
>
> Running the test "Miscellaneous Intel PT testing" with a build with
> -fsanitize=memory and -fsanitize-memory-track-origins I saw:
>
> ```
> ==1257749==WARNING: MemorySanitizer: use-of-uninitialized-value
> #0 0x5581c00a76b3 in intel_pt_sample_flags tools/perf/util/intel-pt.c:1527:17
> #1 0x5581c00c5cf6 in intel_pt_run_decoder tools/perf/util/intel-pt.c:2961:3
> #2 0x5581c00968f8 in intel_pt_process_timeless_queues tools/perf/util/intel-pt.c:3074:4
> #3 0x5581c007cf49 in intel_pt_process_event tools/perf/util/intel-pt.c:3482:10
> #4 0x5581bffa269a in auxtrace__process_event tools/perf/util/auxtracec:2830:9
> #5 0x5581bfb826c0 in perf_session__deliver_event tools/perf/util/session.c:1649:8
> #6 0x5581bfba1d7f in perf_session__process_event tools/perf/util/session.c:1891:9
> #7 0x5581bfba82e4 in process_simple tools/perf/util/session.c:2452:9
> #8 0x5581bfbabcc3 in reader__read_event tools/perf/util/session.c:2381:14
> #9 0x5581bfbad942 in reader__process_events tools/perf/util/session.c:2430:8
> #10 0x5581bfb78256 in __perf_session__process_events tools/perf/util/session.c:2477:8
> #11 0x5581bfb702c4 in perf_session__process_events tools/perf/util/session.c:2643:9
> #12 0x5581bf2da266 in __cmd_script tools/perf/builtin-script.c:2855:8
> #13 0x5581bf2bfcdd in cmd_script tools/perf/builtin-script.c:4402:8
> #14 0x5581bf67004b in run_builtin tools/perf/perf.c:350:11
> #15 0x5581bf66b8ea in handle_internal_command tools/perf/perf.c:403:8
> #16 0x5581bf66f527 in run_argv tools/perf/perf.c:447:2
> #17 0x5581bf669d2d in main tools/perf/perf.c:561:3
>
> Uninitialized value was stored to memory at
> #0 0x5581c005ddf8 in intel_pt_walk_insn tools/perf/util/intel-pt-decoder/intel-pt-decoder.c:1256:25
> #1 0x5581c001a932 in intel_pt_walk_fup tools/perf/util/intel-pt-decoder/intel-pt-decoder.c:1428:9
> #2 0x5581c000f76c in intel_pt_walk_trace tools/perf/util/intel-pt-decoder/intel-pt-decoder.c:3299:10
> #3 0x5581c000899b in intel_pt_decode tools/perf/util/intel-pt-decoder/intel-pt-decoder.c:3988:10
> #4 0x5581c00c5180 in intel_pt_run_decoder tools/perf/util/intel-pt.c:2941:11
> #5 0x5581c00968f8 in intel_pt_process_timeless_queues tools/perf/util/intel-pt.c:3074:4
> #6 0x5581c007cf49 in intel_pt_process_event tools/perf/util/intel-pt.c:3482:10
> #7 0x5581bffa269a in auxtrace__process_event tools/perf/util/auxtracec:2830:9
> #8 0x5581bfb826c0 in perf_session__deliver_event tools/perf/util/session.c:1649:8
> #9 0x5581bfba1d7f in perf_session__process_event tools/perf/util/session.c:1891:9
> #10 0x5581bfba82e4 in process_simple tools/perf/util/session.c:2452:9
> #11 0x5581bfbabcc3 in reader__read_event tools/perf/util/session.c:2381:14
> #12 0x5581bfbad942 in reader__process_events tools/perf/util/session.c:2430:8
> #13 0x5581bfb78256 in __perf_session__process_events tools/perf/util/session.c:2477:8
> #14 0x5581bfb702c4 in perf_session__process_events tools/perf/util/session.c:2643:9
> #15 0x5581bf2da266 in __cmd_script tools/perf/builtin-script.c:2855:8
> #16 0x5581bf2bfcdd in cmd_script tools/perf/builtin-script.c:4402:8
> #17 0x5581bf67004b in run_builtin tools/perf/perf.c:350:11
> #18 0x5581bf66b8ea in handle_internal_command tools/perf/perf.c:403:8
> #19 0x5581bf66f527 in run_argv tools/perf/perf.c:447:2
> ```
>
> Adding a curly brace initializer for the intel_pt_insn in
> intel_pt_walk_fup rectifies this, however, there may be other issues
> lurking behind this so initialize all similar instances.
>
> Fixes: f4aa081949e7 ("perf tools: Add Intel PT decoder")
> Signed-off-by: Ian Rogers <[email protected]>
Adrian, could you take a look at this for the sake of tests passing with msan.
Thanks,
Ian
> ---
> tools/perf/util/intel-pt-decoder/intel-pt-decoder.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
> index b450178e3420..b4a95af2e4cc 100644
> --- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
> +++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
> @@ -1115,7 +1115,7 @@ static void intel_pt_sample_insn(struct intel_pt_decoder *decoder)
> */
> static void intel_pt_sample_fup_insn(struct intel_pt_decoder *decoder)
> {
> - struct intel_pt_insn intel_pt_insn;
> + struct intel_pt_insn intel_pt_insn = {};
> uint64_t max_insn_cnt, insn_cnt = 0;
> int err;
>
> @@ -1418,7 +1418,7 @@ static inline bool intel_pt_fup_with_nlip(struct intel_pt_decoder *decoder,
>
> static int intel_pt_walk_fup(struct intel_pt_decoder *decoder)
> {
> - struct intel_pt_insn intel_pt_insn;
> + struct intel_pt_insn intel_pt_insn = {};
> uint64_t ip;
> int err;
>
> @@ -1461,7 +1461,7 @@ static int intel_pt_walk_fup(struct intel_pt_decoder *decoder)
>
> static int intel_pt_walk_tip(struct intel_pt_decoder *decoder)
> {
> - struct intel_pt_insn intel_pt_insn;
> + struct intel_pt_insn intel_pt_insn = {};
> int err;
>
> err = intel_pt_walk_insn(decoder, &intel_pt_insn, 0);
> @@ -1626,7 +1626,7 @@ static int intel_pt_emulated_ptwrite(struct intel_pt_decoder *decoder)
>
> static int intel_pt_walk_tnt(struct intel_pt_decoder *decoder)
> {
> - struct intel_pt_insn intel_pt_insn;
> + struct intel_pt_insn intel_pt_insn = {};
> int err;
>
> while (1) {
> --
> 2.44.0.291.gc1ea87d7ee-goog
>
On Tue, Mar 26, 2024 at 06:52:12PM +0200, Adrian Hunter wrote:
> On 26/03/24 18:13, Ian Rogers wrote:
> > On Tue, Mar 26, 2024 at 1:32 AM Adrian Hunter <[email protected]> wrote:
> >>
> >> MemorySanitizer discovered instances where the instruction op value was
> >> not assigned.:
> >>
> >> WARNING: MemorySanitizer: use-of-uninitialized-value
> >> #0 0x5581c00a76b3 in intel_pt_sample_flags tools/perf/util/intel-pt.c:1527:17
> >> Uninitialized value was stored to memory at
> >> #0 0x5581c005ddf8 in intel_pt_walk_insn tools/perf/util/intel-pt-decoder/intel-pt-decoder.c:1256:25
> >>
> >> The op value is used to set branch flags for branch instructions
> >> encountered when walking the code, so fix by setting op to
> >> INTEL_PT_OP_OTHER in other cases.
> >>
> >> Reported-by: Ian Rogers <[email protected]>
> >> Closes: https://lore.kernel.org/linux-perf-users/[email protected]/
> >> Signed-off-by: Adrian Hunter <[email protected]>
> >
> > Great, thanks! Should it have a Fixes tag like:
> >
> > Fixes: 4c761d805bb2 ("perf intel-pt: Fix intel_pt_fup_event()
> > assumptions about setting state type")
>
> Yes, I should have put a fixes tag, cc stable. I think the issue
> has always been there, so:
>
> Fixes: 90e457f7be087 ("perf tools: Add Intel PT support")
> Cc: [email protected]
Thanks, applied, had fell thru the cracks :-\
- Arnaldo
> >
> > Tested-by: Ian Rogers <[email protected]>
> >
> > Ian
> >
> >> ---
> >> tools/perf/util/intel-pt-decoder/intel-pt-decoder.c | 2 ++
> >> tools/perf/util/intel-pt.c | 2 ++
> >> 2 files changed, 4 insertions(+)
> >>
> >> diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
> >> index b450178e3420..e733f6b1f7ac 100644
> >> --- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
> >> +++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
> >> @@ -1319,6 +1319,8 @@ static bool intel_pt_fup_event(struct intel_pt_decoder *decoder, bool no_tip)
> >> bool ret = false;
> >>
> >> decoder->state.type &= ~INTEL_PT_BRANCH;
> >> + decoder->state.insn_op = INTEL_PT_OP_OTHER;
> >> + decoder->state.insn_len = 0;
> >>
> >> if (decoder->set_fup_cfe_ip || decoder->set_fup_cfe) {
> >> bool ip = decoder->set_fup_cfe_ip;
> >> diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
> >> index f38893e0b036..4db9a098f592 100644
> >> --- a/tools/perf/util/intel-pt.c
> >> +++ b/tools/perf/util/intel-pt.c
> >> @@ -764,6 +764,7 @@ static int intel_pt_walk_next_insn(struct intel_pt_insn *intel_pt_insn,
> >>
> >> addr_location__init(&al);
> >> intel_pt_insn->length = 0;
> >> + intel_pt_insn->op = INTEL_PT_OP_OTHER;
> >>
> >> if (to_ip && *ip == to_ip)
> >> goto out_no_cache;
> >> @@ -898,6 +899,7 @@ static int intel_pt_walk_next_insn(struct intel_pt_insn *intel_pt_insn,
> >>
> >> if (to_ip && *ip == to_ip) {
> >> intel_pt_insn->length = 0;
> >> + intel_pt_insn->op = INTEL_PT_OP_OTHER;
> >> goto out_no_cache;
> >> }
> >>
> >> --
> >> 2.34.1
> >>
>