Page MenuHomeFreeBSD
Differential D24535

config: Add no-ctfconvert support.
ClosedPublic
Actions

Authored by bdrewery on Apr 22 2020, 6:58 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mar 1 2024, 9:50 PM
Unknown Object (File)
Feb 23 2024, 8:57 AM
Unknown Object (File)
Feb 23 2024, 8:57 AM
Unknown Object (File)
Feb 23 2024, 8:57 AM
Unknown Object (File)
Feb 23 2024, 8:53 AM
Unknown Object (File)
Feb 23 2024, 8:53 AM
Unknown Object (File)
Feb 23 2024, 8:53 AM
Unknown Object (File)
Feb 22 2024, 10:13 PM
View All 63 Files
Subscribers
kevans

Details

Reviewers
imp
cem
markj
kevans
Commits
rS360443: config: Add no-ctfconvert support.
Summary

Some files may purposely have debug info disabled or are *source files*
that attempt to run ctfconvert on them. Currently ctfconvert ignores
these errors but I have a change to make the errors real so we can
catch real problems like exceeding type limits.

I am not sure what to do with the versions/bootstrap here.

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 30703
Build 28433: arc lint + arc unit

Event Timeline

bdrewery created this revision.Apr 22 2020, 6:58 PM
Harbormaster completed remote builds in B30666: Diff 70885.Apr 22 2020, 6:58 PM
bdrewery added a child revision: D24536: Don't try ctfconvert on file without debug info..Apr 22 2020, 7:01 PM
markj accepted this revision.Apr 22 2020, 7:06 PM

It looks right to me. I'm not sure how to handle bootstrapping either.

usr.sbin/config/config.h
85 ↗(On Diff #70885)

Change these to hex while here?

This revision is now accepted and ready to land.Apr 22 2020, 7:06 PM
bdrewery added a child revision: D24537: ctfmerge: Assert that there is enough room for types..Apr 22 2020, 7:08 PM
cem accepted this revision.Apr 23 2020, 2:16 AM
cem added a subscriber: kevans.

I think Kyle has looked at config(8) somewhat recently, maybe he has some insight here.

usr.sbin/config/mkmakefile.c
601–602 ↗(On Diff #70885)

Could just set NO_OBJ | NO_CTFCONVERT here.

815 ↗(On Diff #70885)

And then this test would just be for NO_CTFCONVERT, which seems reasonable given what the branch does. Not a big deal.

kevans added a comment.Apr 23 2020, 2:41 AM

This should entail a CONFIGVERS bump to 600018 with a %VERSREQ bump to that version in all affected sys/conf/Makefile.* when a change is made that requires the new no-ctfconvert syntax. Tagging in @imp , though.

bdrewery updated this revision to Diff 70948.Apr 24 2020, 5:22 PM

Bump CONFIGVERS for change

This revision now requires review to proceed.Apr 24 2020, 5:22 PM
Harbormaster completed remote builds in B30703: Diff 70948.Apr 24 2020, 5:22 PM
bdrewery updated this revision to Diff 70949.Apr 24 2020, 5:22 PM

Upload proper diff

Harbormaster completed remote builds in B30704: Diff 70949.Apr 24 2020, 5:22 PM
cem accepted this revision.Apr 24 2020, 5:28 PM
This revision is now accepted and ready to land.Apr 24 2020, 5:28 PM
kevans accepted this revision.Apr 24 2020, 5:30 PM
bdrewery added inline comments.Apr 27 2020, 5:06 PM
usr.sbin/config/mkmakefile.c
170–174 ↗(On Diff #70949)

@cem NO_OBJ has more behavior than NO_CTFCONVERT down in do_objs.

674–676 ↗(On Diff #70949)

@cem NO_OBJ has more behavior than NO_CTFCONVERT down in do_objs.

imp accepted this revision.Apr 27 2020, 5:18 PM

Will need to remember to bump config version for anything that uses a driver that's tagged with this.

cem added inline comments.Apr 28 2020, 1:26 AM
usr.sbin/config/mkmakefile.c
674–676 ↗(On Diff #70949)

I don't think that's responsive to my suggestion, or at least I'm not seeing it.

The idea would be: "no-obj" sets both the NO_OBJ and NO_CTFCONVERT flags.

Then when we decide whether or not to emit "NORMAL_CTFCONVERT", we choose only on the basis of the NO_CTFCONVERT flag.

The only way this would break existing NO_OBJ logic is if something does an equality test for NO_OBJ rather than a flag bit test. If something does, I think it's already broken (given other attributes may be set already).

bdrewery added inline comments.Apr 28 2020, 3:26 PM
usr.sbin/config/mkmakefile.c
674–676 ↗(On Diff #70949)

I misinterpreted your suggestion as setting NO_OBJ with no-ctfconvert. I get it now.

bdrewery updated this revision to Diff 71108.Apr 28 2020, 3:29 PM

Set NO_CTFCONVERT for no-obj

This revision now requires review to proceed.Apr 28 2020, 3:29 PM
Harbormaster completed remote builds in B30777: Diff 71108.Apr 28 2020, 3:29 PM
cem accepted this revision.Apr 28 2020, 4:05 PM

Thanks! That's exactly what I had in mind :-).

This revision is now accepted and ready to land.Apr 28 2020, 4:05 PM
Closed by commit rS360443: config: Add no-ctfconvert support. (authored by bdrewery). · Explain WhyApr 28 2020, 4:09 PM
This revision was automatically updated to reflect the committed changes.
bdrewery added a commit: rS360443: config: Add no-ctfconvert support..

Revision Contents
Changeset List

PathSize
cddl/
contrib/
opensolaris/
tools/
ctf/
cvt/
8 lines
10 lines
sys/
conf/
2 lines
3 lines

Diff 70948

View Options

cddl/contrib/opensolaris/tools/ctf/cvt/merge.c

Loading...
View Options

cddl/contrib/opensolaris/tools/ctf/cvt/util.c

Loading...
View Options

sys/conf/Makefile.amd64

Loading...
View Options

sys/conf/files.amd64

Loading...