git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] Start compiling with `-Wsign-compare`
@ 2024-11-29 13:13 Patrick Steinhardt
  2024-11-29 13:13 ` [PATCH 01/10] git-compat-util: introduce macros to disable "-Wsign-compare" warnings Patrick Steinhardt
                   ` (14 more replies)
  0 siblings, 15 replies; 87+ messages in thread
From: Patrick Steinhardt @ 2024-11-29 13:13 UTC (permalink / raw)
  To: git

Hi,

when compiling with DEVELOPER=YesPlease, we explicitly disable the
"-Wsign-compare" warning. This is mostly because our code base is full
of cases where we don't bother at all whether something should be signed
or unsigned, and enabling the warning would thus cause tons of warnings
to pop up.

Unfortunately, disabling this warning also masks real issues. There have
been multiple CVEs in the Git project that would have been flagged by
this warning (e.g. CVE-2022-39260, CVE-2022-41903 and several fixes in
the vicinity of these CVEs). Furthermore, the final audit report by
X41 D-Sec, who are the ones who have discovered some of the CVEs, hinted
that it might be a good idea to become more strict in this context.

Now simply enabling the warning globally does not fly due to the stated
reason above that we simply have too many sites where we use the wrong
integer types. Instead, this patch series introduces a new macro that
allows us to explicitly mark files that generate such warnings. Like
this, we can adapt the codebase over time and hopefully make this class
of vulnerabilities harder to land.

There are a couple of trivial conflicts with kn/midx-wo-the-repository,
but I don't think it makes sense to make that a dependency of this
sereis. Let me know in case you disagree and I'll change the base of
this series.

Thanks!

Patrick

---
Patrick Steinhardt (10):
      git-compat-util: introduce macros to disable "-Wsign-compare" warnings
      compat/regex: explicitly ignore "-Wsign-compare" warnings
      compat/win32: fix -Wsign-compare warning in "wWinMain()"
      global: mark code units that generate warnings with `-Wsign-compare`
      config.mak.dev: drop `-Wno-sign-compare`
      global: fix unsigned integer promotions in ternary statements
      diff.h: fix index used to loop through unsigned integer
      global: trivial conversions to fix `-Wsign-compare` warnings
      daemon: fix loops that have mismatching integer types
      daemon: fix type of `max_connections`

 add-interactive.c                    |  1 +
 add-patch.c                          |  1 +
 advice.c                             |  7 ++-----
 apply.c                              |  1 +
 archive.c                            |  1 +
 attr.c                               |  1 +
 base85.c                             |  3 +--
 bisect.c                             |  1 +
 blame.c                              |  1 +
 bloom.c                              |  2 ++
 builtin/add.c                        |  9 +++++----
 builtin/am.c                         |  1 +
 builtin/bisect.c                     |  2 ++
 builtin/blame.c                      |  5 ++++-
 builtin/branch.c                     |  2 ++
 builtin/cat-file.c                   |  3 +++
 builtin/checkout--worker.c           |  2 ++
 builtin/checkout-index.c             |  3 +++
 builtin/checkout.c                   |  2 ++
 builtin/clean.c                      |  3 +++
 builtin/clone.c                      |  3 +++
 builtin/commit.c                     |  3 +++
 builtin/describe.c                   |  2 ++
 builtin/diff-files.c                 |  3 +++
 builtin/diff-index.c                 |  2 ++
 builtin/diff-tree.c                  |  1 +
 builtin/diff.c                       |  3 +++
 builtin/difftool.c                   |  5 ++++-
 builtin/fast-export.c                |  3 +++
 builtin/fast-import.c                |  2 ++
 builtin/fetch-pack.c                 |  2 ++
 builtin/fetch.c                      |  3 +++
 builtin/for-each-repo.c              |  5 +++--
 builtin/fsmonitor--daemon.c          |  2 ++
 builtin/gc.c                         |  3 +++
 builtin/grep.c                       |  3 +++
 builtin/help.c                       |  6 +++---
 builtin/index-pack.c                 |  2 ++
 builtin/log.c                        |  3 +++
 builtin/ls-files.c                   |  3 +++
 builtin/mailsplit.c                  |  6 ++++--
 builtin/merge-file.c                 |  2 ++
 builtin/merge-index.c                |  2 ++
 builtin/merge-ours.c                 |  2 ++
 builtin/merge-tree.c                 |  6 +++---
 builtin/merge.c                      |  3 +++
 builtin/mv.c                         |  2 ++
 builtin/name-rev.c                   |  2 ++
 builtin/pack-objects.c               |  2 ++
 builtin/pack-redundant.c             |  4 ++--
 builtin/pack-refs.c                  |  1 +
 builtin/patch-id.c                   |  3 ++-
 builtin/prune.c                      |  2 ++
 builtin/pull.c                       |  4 ++--
 builtin/push.c                       |  6 ++++--
 builtin/range-diff.c                 |  1 +
 builtin/rebase.c                     |  3 +++
 builtin/receive-pack.c               |  2 ++
 builtin/reflog.c                     |  1 +
 builtin/remote.c                     |  2 ++
 builtin/repack.c                     |  2 ++
 builtin/replay.c                     |  4 +++-
 builtin/rerere.c                     |  9 +++++----
 builtin/reset.c                      |  2 ++
 builtin/rev-list.c                   |  2 ++
 builtin/rev-parse.c                  |  3 +++
 builtin/revert.c                     |  1 +
 builtin/rm.c                         |  3 +++
 builtin/shortlog.c                   |  1 +
 builtin/show-branch.c                |  2 ++
 builtin/show-index.c                 |  2 ++
 builtin/sparse-checkout.c            |  2 ++
 builtin/stash.c                      |  7 +++----
 builtin/submodule--helper.c          |  8 ++++----
 builtin/tag.c                        |  3 +++
 builtin/unpack-objects.c             |  2 ++
 builtin/update-index.c               |  3 +++
 builtin/update-ref.c                 |  2 ++
 builtin/var.c                        |  5 +++--
 builtin/worktree.c                   |  2 ++
 bulk-checkin.c                       |  1 +
 bundle-uri.c                         |  1 +
 bundle.c                             |  1 +
 cache-tree.c                         |  1 +
 chunk-format.c                       |  1 +
 color.c                              |  2 ++
 column.c                             |  2 ++
 combine-diff.c                       |  1 +
 commit-graph.c                       |  1 +
 commit-reach.c                       |  1 +
 commit.c                             |  3 +--
 compat/fsmonitor/fsm-listen-darwin.c |  3 +--
 compat/mingw.c                       |  1 +
 compat/poll/poll.c                   |  2 ++
 compat/regex/regex.c                 |  2 ++
 compat/terminal.c                    |  3 +--
 compat/win32/headless.c              | 17 +++++++++--------
 compat/win32mmap.c                   |  2 ++
 compat/winansi.c                     |  2 ++
 config.c                             |  1 +
 config.mak.dev                       |  1 -
 connect.c                            |  1 +
 convert.c                            |  1 +
 credential.c                         |  1 +
 daemon.c                             | 31 +++++++++++++------------------
 date.c                               |  2 ++
 decorate.c                           |  3 +++
 delta-islands.c                      |  1 +
 diagnose.c                           |  7 +++----
 diff-delta.c                         |  2 ++
 diff-lib.c                           |  1 +
 diff-no-index.c                      |  2 ++
 diff.c                               |  1 +
 diff.h                               |  3 +--
 diffcore-order.c                     |  1 +
 diffcore-pickaxe.c                   |  3 +++
 diffcore-rename.c                    |  3 +--
 diffcore-rotate.c                    |  1 +
 dir.c                                |  5 +++--
 entry.c                              |  4 ++--
 ewah/ewah_bitmap.c                   |  5 ++---
 ewah/ewah_io.c                       |  3 +++
 ewah/ewah_rlw.c                      |  3 +++
 fetch-pack.c                         |  1 +
 fmt-merge-msg.c                      |  1 +
 fsmonitor.c                          |  1 +
 gettext.c                            |  2 +-
 git-compat-util.h                    | 10 ++++++++++
 git.c                                | 20 +++++++-------------
 gpg-interface.c                      | 14 ++++++--------
 graph.c                              |  1 +
 grep.c                               |  2 ++
 help.c                               |  1 +
 help.h                               |  8 +++-----
 hex.c                                |  6 ++----
 http-backend.c                       |  1 +
 http-push.c                          |  3 +--
 http-walker.c                        |  1 +
 http.c                               |  1 +
 imap-send.c                          |  1 +
 json-writer.c                        |  2 ++
 kwset.c                              |  2 ++
 line-log.c                           |  2 ++
 list-objects-filter-options.c        |  4 +---
 list-objects.c                       |  7 ++-----
 log-tree.c                           |  1 +
 ls-refs.c                            |  4 +---
 mailinfo.c                           |  1 +
 mailmap.c                            |  1 +
 match-trees.c                        |  1 +
 mem-pool.c                           |  2 ++
 merge-ll.c                           |  1 +
 merge-ort.c                          |  1 +
 merge-recursive.c                    |  1 +
 merge.c                              |  4 ++--
 midx-write.c                         |  1 +
 midx.c                               |  1 +
 name-hash.c                          |  1 +
 notes-merge.c                        |  1 +
 notes.c                              |  1 +
 object-file-convert.c                |  1 +
 object-file.c                        |  1 +
 object-name.c                        |  1 +
 object.c                             |  1 +
 pack-bitmap-write.c                  |  1 +
 pack-bitmap.c                        |  1 +
 pack-check.c                         |  1 +
 packfile.c                           |  1 +
 parallel-checkout.c                  |  1 +
 path.c                               |  4 ++--
 pathspec.c                           |  1 +
 pkt-line.c                           |  3 +--
 preload-index.c                      |  1 +
 pretty.c                             |  1 +
 progress.c                           |  1 +
 pseudo-merge.c                       |  1 +
 quote.c                              |  2 ++
 range-diff.c                         |  1 +
 read-cache.c                         |  1 +
 ref-filter.c                         |  1 +
 reflog.c                             |  1 +
 refs.c                               |  1 +
 refs/debug.c                         |  3 +--
 refs/files-backend.c                 |  1 +
 refs/iterator.c                      |  2 ++
 refs/packed-backend.c                |  1 +
 refspec.c                            |  1 +
 reftable/system.h                    |  2 ++
 remote-curl.c                        |  1 +
 remote.c                             |  1 +
 rerere.c                             |  1 +
 resolve-undo.c                       |  1 +
 revision.c                           |  1 +
 run-command.c                        |  1 +
 scalar.c                             |  6 +++---
 send-pack.c                          |  5 ++---
 sequencer.c                          |  1 +
 serve.c                              |  7 ++-----
 server-info.c                        |  1 +
 setup.c                              |  1 +
 shallow.c                            |  1 +
 sideband.c                           |  1 +
 sparse-index.c                       |  1 +
 split-index.c                        |  1 +
 strbuf.c                             |  4 +++-
 string-list.c                        |  2 ++
 strvec.c                             |  3 +--
 submodule-config.c                   |  1 +
 submodule.c                          |  1 +
 symlinks.c                           |  2 ++
 t/helper/test-bloom.c                |  9 ++-------
 t/helper/test-cache-tree.c           |  1 +
 t/helper/test-config.c               |  1 +
 t/helper/test-csprng.c               |  2 +-
 t/helper/test-drop-caches.c          |  2 ++
 t/helper/test-dump-fsmonitor.c       |  3 +--
 t/helper/test-dump-split-index.c     |  3 +--
 t/helper/test-dump-untracked-cache.c |  6 +++---
 t/helper/test-genrandom.c            |  2 +-
 t/helper/test-genzeros.c             |  2 ++
 t/helper/test-hash-speed.c           |  5 ++---
 t/helper/test-mergesort.c            |  2 ++
 t/helper/test-parse-options.c        |  5 ++---
 t/helper/test-path-utils.c           |  1 +
 t/helper/test-reach.c                |  3 +--
 t/helper/test-ref-store.c            |  3 +--
 t/helper/test-run-command.c          |  2 ++
 t/helper/test-string-list.c          |  2 ++
 t/helper/test-tool.c                 |  3 +--
 t/helper/test-trace2.c               |  1 +
 t/unit-tests/lib-reftable.c          |  2 ++
 t/unit-tests/t-example-decorate.c    |  4 ++--
 t/unit-tests/t-prio-queue.c          |  2 +-
 t/unit-tests/t-reftable-readwrite.c  |  2 ++
 t/unit-tests/t-reftable-stack.c      |  2 ++
 t/unit-tests/t-trailer.c             |  2 ++
 t/unit-tests/test-lib.c              |  2 ++
 tag.c                                |  1 +
 tmp-objdir.c                         |  3 +--
 trace.c                              |  1 +
 trace2.c                             |  2 ++
 trace2/tr2_sysenv.c                  |  2 ++
 trace2/tr2_tgt_event.c               |  2 ++
 trace2/tr2_tgt_perf.c                |  2 ++
 trailer.c                            |  3 +--
 transport-helper.c                   | 13 ++++++-------
 transport.c                          | 13 ++++---------
 tree-diff.c                          |  1 +
 unix-socket.c                        |  2 ++
 unpack-trees.c                       |  1 +
 upload-pack.c                        |  1 +
 urlmatch.c                           |  2 ++
 usage.c                              |  3 ++-
 userdiff.c                           |  1 +
 utf8.c                               |  2 ++
 version.c                            |  3 +--
 versioncmp.c                         |  3 +--
 worktree.c                           |  1 +
 wrapper.c                            |  3 +++
 ws.c                                 |  3 +++
 wt-status.c                          |  1 +
 xdiff-interface.c                    |  1 +
 xdiff/xdiffi.c                       |  1 +
 xdiff/xinclude.h                     |  2 ++
 264 files changed, 491 insertions(+), 215 deletions(-)


---
base-commit: cc01bad4a9f566cf4453c7edd6b433851b0835e2
change-id: 20241128-pks-sign-compare-9cf7412bce1a


^ permalink raw reply	[flat|nested] 87+ messages in thread

end of thread, other threads:[~2025-01-09  6:20 UTC | newest]

Thread overview: 87+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-29 13:13 [PATCH 00/10] Start compiling with `-Wsign-compare` Patrick Steinhardt
2024-11-29 13:13 ` [PATCH 01/10] git-compat-util: introduce macros to disable "-Wsign-compare" warnings Patrick Steinhardt
2024-11-29 13:13 ` [PATCH 02/10] compat/regex: explicitly ignore " Patrick Steinhardt
2024-11-29 13:13 ` [PATCH 03/10] compat/win32: fix -Wsign-compare warning in "wWinMain()" Patrick Steinhardt
2024-11-29 13:13 ` [PATCH 04/10] global: mark code units that generate warnings with `-Wsign-compare` Patrick Steinhardt
2024-11-29 13:13 ` [PATCH 05/10] config.mak.dev: drop `-Wno-sign-compare` Patrick Steinhardt
2024-11-29 13:13 ` [PATCH 06/10] global: fix unsigned integer promotions in ternary statements Patrick Steinhardt
2024-11-30 10:44   ` shejialuo
2024-12-02  7:54     ` Patrick Steinhardt
2024-12-01 21:59   ` Jeff King
2024-12-02  7:54     ` Patrick Steinhardt
2024-11-29 13:13 ` [PATCH 07/10] diff.h: fix index used to loop through unsigned integer Patrick Steinhardt
2024-11-29 13:13 ` [PATCH 08/10] global: trivial conversions to fix `-Wsign-compare` warnings Patrick Steinhardt
2024-12-01 22:07   ` Jeff King
2024-12-02  7:54     ` Patrick Steinhardt
2024-11-29 13:13 ` [PATCH 09/10] daemon: fix loops that have mismatching integer types Patrick Steinhardt
2024-12-01 22:08   ` Jeff King
2024-12-02  7:54     ` Patrick Steinhardt
2024-12-05 19:14       ` Jeff King
2024-11-29 13:13 ` [PATCH 10/10] daemon: fix type of `max_connections` Patrick Steinhardt
2024-12-01 22:09   ` Jeff King
2024-11-30 10:55 ` [PATCH 00/10] Start compiling with `-Wsign-compare` shejialuo
2024-12-02  7:54   ` Patrick Steinhardt
2024-12-01 22:29 ` Jeff King
2024-12-02  7:53   ` Patrick Steinhardt
2024-12-02 12:04 ` [PATCH v2 00/14] " Patrick Steinhardt
2024-12-02 12:04   ` [PATCH v2 01/14] git-compat-util: introduce macros to disable "-Wsign-compare" warnings Patrick Steinhardt
2024-12-02 12:04   ` [PATCH v2 02/14] compat/regex: explicitly ignore " Patrick Steinhardt
2024-12-02 12:04   ` [PATCH v2 03/14] compat/win32: fix -Wsign-compare warning in "wWinMain()" Patrick Steinhardt
2024-12-02 12:04   ` [PATCH v2 04/14] global: mark code units that generate warnings with `-Wsign-compare` Patrick Steinhardt
2024-12-02 12:04   ` [PATCH v2 05/14] config.mak.dev: drop `-Wno-sign-compare` Patrick Steinhardt
2024-12-02 12:04   ` [PATCH v2 06/14] diff.h: fix index used to loop through unsigned integer Patrick Steinhardt
2024-12-02 12:04   ` [PATCH v2 07/14] global: trivial conversions to fix `-Wsign-compare` warnings Patrick Steinhardt
2024-12-04  5:31     ` Junio C Hamano
2024-12-02 12:04   ` [PATCH v2 08/14] daemon: fix loops that have mismatching integer types Patrick Steinhardt
2024-12-02 12:04   ` [PATCH v2 09/14] daemon: fix type of `max_connections` Patrick Steinhardt
2024-12-02 12:04   ` [PATCH v2 10/14] gpg-interface: address -Wsign-comparison warnings Patrick Steinhardt
2024-12-02 12:04   ` [PATCH v2 11/14] builtin/blame: fix type of `length` variable when emitting object ID Patrick Steinhardt
2024-12-02 12:04   ` [PATCH v2 12/14] builtin/patch-id: fix type of `get_one_patchid()` Patrick Steinhardt
2024-12-02 13:18     ` shejialuo
2024-12-02 13:24       ` Patrick Steinhardt
2024-12-02 12:04   ` [PATCH v2 13/14] scalar: address -Wsign-compare warnings Patrick Steinhardt
2024-12-02 12:04   ` [PATCH v2 14/14] t/helper: don't depend on implicit wraparound Patrick Steinhardt
2024-12-02 13:28   ` [PATCH v2 00/14] Start compiling with `-Wsign-compare` shejialuo
2024-12-04  5:47   ` [PATCH] sign-compare: 32-bit support Junio C Hamano
2024-12-05  9:32     ` Patrick Steinhardt
2024-12-05  9:36 ` [PATCH v3 00/15] Start compiling with `-Wsign-compare` Patrick Steinhardt
2024-12-05  9:36   ` [PATCH v3 01/15] git-compat-util: introduce macros to disable "-Wsign-compare" warnings Patrick Steinhardt
2024-12-05  9:36   ` [PATCH v3 02/15] compat/regex: explicitly ignore " Patrick Steinhardt
2024-12-05  9:36   ` [PATCH v3 03/15] compat/win32: fix -Wsign-compare warning in "wWinMain()" Patrick Steinhardt
2024-12-05  9:36   ` [PATCH v3 04/15] global: mark code units that generate warnings with `-Wsign-compare` Patrick Steinhardt
2024-12-05  9:36   ` [PATCH v3 05/15] config.mak.dev: drop `-Wno-sign-compare` Patrick Steinhardt
2024-12-05  9:36   ` [PATCH v3 06/15] diff.h: fix index used to loop through unsigned integer Patrick Steinhardt
2024-12-05  9:36   ` [PATCH v3 07/15] sign-compare: 32-bit support Patrick Steinhardt
2024-12-05 19:34     ` Jeff King
2024-12-06  8:44       ` Patrick Steinhardt
2024-12-05  9:36   ` [PATCH v3 08/15] global: trivial conversions to fix `-Wsign-compare` warnings Patrick Steinhardt
2024-12-05  9:36   ` [PATCH v3 09/15] daemon: fix loops that have mismatching integer types Patrick Steinhardt
2024-12-05  9:36   ` [PATCH v3 10/15] daemon: fix type of `max_connections` Patrick Steinhardt
2024-12-05  9:36   ` [PATCH v3 11/15] gpg-interface: address -Wsign-comparison warnings Patrick Steinhardt
2024-12-05  9:36   ` [PATCH v3 12/15] builtin/blame: fix type of `length` variable when emitting object ID Patrick Steinhardt
2024-12-05  9:36   ` [PATCH v3 13/15] builtin/patch-id: fix type of `get_one_patchid()` Patrick Steinhardt
2024-12-05  9:36   ` [PATCH v3 14/15] scalar: address -Wsign-compare warnings Patrick Steinhardt
2024-12-05  9:36   ` [PATCH v3 15/15] t/helper: don't depend on implicit wraparound Patrick Steinhardt
2024-12-06 10:27 ` [PATCH v4 00/16] Start compiling with `-Wsign-compare` Patrick Steinhardt
2024-12-06 10:27   ` [PATCH v4 01/16] git-compat-util: introduce macros to disable "-Wsign-compare" warnings Patrick Steinhardt
2024-12-06 12:32     ` karthik nayak
2024-12-06 10:27   ` [PATCH v4 02/16] compat/regex: explicitly ignore " Patrick Steinhardt
2024-12-06 10:27   ` [PATCH v4 03/16] compat/win32: fix -Wsign-compare warning in "wWinMain()" Patrick Steinhardt
2024-12-06 10:27   ` [PATCH v4 04/16] global: mark code units that generate warnings with `-Wsign-compare` Patrick Steinhardt
2024-12-06 10:27   ` [PATCH v4 05/16] config.mak.dev: drop `-Wno-sign-compare` Patrick Steinhardt
2024-12-06 10:27   ` [PATCH v4 06/16] diff.h: fix index used to loop through unsigned integer Patrick Steinhardt
2024-12-06 10:27   ` [PATCH v4 07/16] csum-file: fix -Wsign-compare warning on 32-bit platform Patrick Steinhardt
2024-12-06 10:27   ` [PATCH v4 08/16] pkt-line: fix -Wsign-compare warning on 32 bit platform Patrick Steinhardt
2024-12-08 19:57     ` Jeff King
2024-12-09  0:09       ` Junio C Hamano
2024-12-06 10:27   ` [PATCH v4 09/16] global: trivial conversions to fix `-Wsign-compare` warnings Patrick Steinhardt
2024-12-06 10:27   ` [PATCH v4 10/16] daemon: fix loops that have mismatching integer types Patrick Steinhardt
2024-12-06 10:27   ` [PATCH v4 11/16] daemon: fix type of `max_connections` Patrick Steinhardt
2024-12-06 10:27   ` [PATCH v4 12/16] gpg-interface: address -Wsign-comparison warnings Patrick Steinhardt
2024-12-06 10:27   ` [PATCH v4 13/16] builtin/blame: fix type of `length` variable when emitting object ID Patrick Steinhardt
2025-01-08 19:17     ` Johannes Schindelin
2025-01-09  6:20       ` Patrick Steinhardt
2024-12-06 10:27   ` [PATCH v4 14/16] builtin/patch-id: fix type of `get_one_patchid()` Patrick Steinhardt
2024-12-06 10:27   ` [PATCH v4 15/16] scalar: address -Wsign-compare warnings Patrick Steinhardt
2024-12-06 10:27   ` [PATCH v4 16/16] t/helper: don't depend on implicit wraparound Patrick Steinhardt
2024-12-06 13:11   ` [PATCH v4 00/16] Start compiling with `-Wsign-compare` karthik nayak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).