Opened 3 months ago

Closed 6 weeks ago

#31354 closed defect (fixed)

Compiler "note" in test_addr.c: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.4.2.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: regression, 042-should, asn-merge
Cc: Actual Points:
Parent ID: Points:
Reviewer: catalyst Sponsor:

Description

Building with GCC 9.1.1, I see these:

src/test/test_addr.c: In function ‘test_addr_parse’:
src/test/test_addr.c:1167:1: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
 1167 | test_addr_parse(void *arg)
      | ^~~~~~~~~~~~~~~

This seems to be new in 0.4.2.

Child Tickets

Change History (14)

comment:1 Changed 7 weeks ago by nickm

Keywords: regression added

Add keyword to unmarked regression defects in 0.4.2.

comment:2 Changed 7 weeks ago by nickm

Owner: set to nickm
Status: newaccepted

This has been bugging me.

comment:3 Changed 7 weeks ago by catalyst

#30968 is related

comment:4 Changed 7 weeks ago by nickm

Hm. My basic fix here will not solve #30968, but it might still be useful under the theory that we shouldn't acclimate people to ignoring warnings, even if the compiler calls those warnings "notes".

Branch is ticket31354, which adopts a brute-force solution to solving the problem (reducing variables and splitting test functions). I didn't do a changes file here, since AFAICT 0.4.1.x doesn't have this problem.

https://github.com/torproject/tor/pull/1299 is the PR. I'll put this in needs_review once it passes CI.

comment:5 Changed 7 weeks ago by nickm

Status: acceptedneeds_review

I think this is ready for a review. It's not the long-term solution, but it may help.

comment:6 Changed 6 weeks ago by dgoulet

Reviewer: #31354

comment:7 Changed 6 weeks ago by dgoulet

Reviewer: #31354catalyst

comment:8 in reply to:  5 Changed 6 weeks ago by catalyst

Status: needs_reviewneeds_revision

Replying to nickm:

I think this is ready for a review. It's not the long-term solution, but it may help.

Thanks! Mostly looks good. I left a comment on the pull request about an extraneous addition.

comment:9 Changed 6 weeks ago by nickm

Status: needs_revisionneeds_review

wow! Sorry about that-- I honestly don't know what I was working on when I left that in.

I've added a fixup commit to the branch.

comment:10 in reply to:  9 Changed 6 weeks ago by catalyst

Status: needs_reviewmerge_ready

Replying to nickm:

wow! Sorry about that-- I honestly don't know what I was working on when I left that in.

I've added a fixup commit to the branch.

Looks good. Thanks!

comment:11 Changed 6 weeks ago by nickm

Mark some merge_ready tickets as 042-should

comment:12 Changed 6 weeks ago by nickm

Keywords: 042-should added

comment:13 Changed 6 weeks ago by nickm

Keywords: asn-merge added

comment:14 Changed 6 weeks ago by asn

Resolution: fixed
Status: merge_readyclosed

Merged!

Note: See TracTickets for help on using tickets.