1393498 - gcc throws noreturn warning about stub_BaseThreadInitThunk in WindowsDllBlocklist.cpp

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Tom Ritter [:tjr]]

The pointer to stub_BaseThreadInitThunk is not marked MOZ_NORETURN so gcc throws a warning when it is called in a MOZ_NORETURN function.

Comment 1

[Tom Ritter [:tjr]]

Attachment: Bug 1393498 Mark the stub_BaseThreadInitThunk as MOZ_NORETURN to silence a warning about a noreturn function not returning

/home/worker/workspace/build/src/mozglue/build/WindowsDllBlocklist.cpp:816:1: error: 'noreturn' function does return [-Werror]

Review commit: https://reviewboard.mozilla.org/r/172230/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/172230/

Comment 2

[Tom Ritter [:tjr]]

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7c5475a142da62b27434c8019545d78902c60e0

(Build will fail but the warning should be gone.)

Comment 3

[(Away)]

Comment on attachment 8900794 [details]
Bug 1393498 Mark the stub_BaseThreadInitThunk as MOZ_NORETURN to silence a warning about a noreturn function not returning

https://reviewboard.mozilla.org/r/172230/#review177520

Would it be better to stick MOZ_NORETURN on the typedef? I'm fine either way.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0df56be60d07
Mark the stub_BaseThreadInitThunk as MOZ_NORETURN to silence a warning about a noreturn function not returning r=dmajor

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back this out for Windows build failures like https://treeherder.mozilla.org/logviewer.html#?job_id=125645448&repo=autoland

Comment 6

[Tom Ritter [:tjr]]

Comment on attachment 8900794 [details]
Bug 1393498 Mark the stub_BaseThreadInitThunk as MOZ_NORETURN to silence a warning about a noreturn function not returning

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/172230/diff/1-2/

Comment 7

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/88dfc4e305fb
Backed out changeset 0df56be60d07 for windows build failures a=backout

Comment 8

[Tom Ritter [:tjr]]

New Try Run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=557f76305e1487dce26ded7a6ac08427537c9511

Comment 9

[Tom Ritter [:tjr]]

Comment on attachment 8900794 [details]
Bug 1393498 Mark the stub_BaseThreadInitThunk as MOZ_NORETURN to silence a warning about a noreturn function not returning

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/172230/diff/2-3/

Comment 10

[Tom Ritter [:tjr]]

This try run puts it on the typedef:    https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf271185a0479cd5b406e837fda5693b2feaab60

Comment 11

[Tom Ritter [:tjr]]

Comment on attachment 8900794 [details]
Bug 1393498 Mark the stub_BaseThreadInitThunk as MOZ_NORETURN to silence a warning about a noreturn function not returning

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/172230/diff/3-4/

Comment 12

[Tom Ritter [:tjr]]

Okay https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf271185a0479cd5b406e837fda5693b2feaab60&selectedJob=125666481 shows a successful run with the attribute on the typedef. I cleaned up the patch and think it is ready for re-review and merge.

Comment 13

[(Away)]

Comment on attachment 8900794 [details]
Bug 1393498 Mark the stub_BaseThreadInitThunk as MOZ_NORETURN to silence a warning about a noreturn function not returning

https://reviewboard.mozilla.org/r/172230/#review177638

Comment 14

[(Away)]

I am surprised that the attribute goes "on the outside", rather than next to __fastcall, but as long as the compiler isn't complaining, ship it!

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d44d3fae6125
Mark the stub_BaseThreadInitThunk as MOZ_NORETURN to silence a warning about a noreturn function not returning r=dmajor

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Backed out for bustage. Please provide a link to a green Try push with the next checkin-needed request.
https://hg.mozilla.org/integration/autoland/rev/9ed004b33828

https://treeherder.mozilla.org/logviewer.html#?job_id=125717808&repo=autoland

Comment 17

[Tom Ritter [:tjr]]

Comment on attachment 8900794 [details]
Bug 1393498 Mark the stub_BaseThreadInitThunk as MOZ_NORETURN to silence a warning about a noreturn function not returning

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/172230/diff/4-5/

Comment 18

[Tom Ritter [:tjr]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #16)
> Backed out for bustage. Please provide a link to a green Try push with the
> next checkin-needed request.

Sorry, it busts in debug but not in opt (and I had only run opt in Comment 12). 

Extensive try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a1e66398566590e496399f4c87118ed62ff27380

dmajor: I fixed this with a new MOZ_ attribute so I'm going to ask you to re-review.

Comment 19

[Tom Ritter [:tjr]]

Trychooser told me about a platform TC throws an error about. Hopefully this one works: https://treeherder.mozilla.org/#/jobs?repo=try&revision=086706e1a7995c0058cb13a91f32ad0a5f8de86e

Comment 20

[Nathan Froyd [:froydnj]]

Comment on attachment 8900794 [details]
Bug 1393498 Mark the stub_BaseThreadInitThunk as MOZ_NORETURN to silence a warning about a noreturn function not returning

https://reviewboard.mozilla.org/r/172230/#review179020

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1499dc6c1769
Mark the stub_BaseThreadInitThunk as MOZ_NORETURN to silence a warning about a noreturn function not returning r=dmajor,froydnj

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/defa8a301a0a
Mark the stub_BaseThreadInitThunk as MOZ_NORETURN to silence a warning about a noreturn function not returning r=dmajor,froydnj

Comment 23

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/defa8a301a0a