Fix GCC v13 LTO build [-Walloc-size-larger-than=] #1929

juju4 · 2024-10-31T17:48:19Z

store/Disks.cc:690: error: argument 1 value 18446744073709551615
    exceeds maximum object size 9223372036854775807
    [-Werror=alloc-size-larger-than=]
const auto tmp = new SwapDir::Pointer[swap->n_allocated];

pconn.cc:43:53: error: argument 1 value 18446744073709551615 ...
theList_ = new Comm::ConnectionPointer[capacity_];

Tested on Ubuntu 24.04 and GCC v13.2.0.

rousskov · 2024-10-31T18:45:33Z

Thank you for improving so many declarations! Please add your contact info to CONTRIBUTORS file.

Work in progress.

I marked this PR as Draft and adjusted PR title/description (i.e. future master commit message) for consistency with similar fixes. PR description will need to be adjusted further to remove temporary discussion and to explain why we are making so many seemingly unrelated changes.

BTW, how did you decide which types/expressions to change? Do all of the proposed changes cascade from fixing the original compiler warning that is now quoted in this PR description? In other words, can any one of the proposed changes be undone without harming the build?

After you are done, can we enable -Walloc-size-larger-than (where supported)? If yes, please add that check to configure.ac. Look for useful warnings that may not be included in -Wall -Wextra text there. Or is that check already enabled by default with sufficient than value to trigger the actual build error?? From this and earlier PR descriptions, I could not tell whether Squid GCC v12 build fails even without that check being explicitly enabled.

store.cc: In member function ‘void StoreEntry::checkDisk() const’:
store.cc:1956:28: error: comparison of integer expressions of different signedness: ‘const signed char:7’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
1956 |             Must(swap_dirn < Config.cacheSwap.n_configured);

extra error pending. I would think changing dirno type to size_t below is not right. What is preferred? casting, changing condition, something else... https://github.com/squid-cache/squid/blob/master/src/store.cc#L1969

You are right -- we should not change StoreEntry::swap_dirn to size_t. Ideally, swap_filen and swap_dirn should probably be wrapped inside something like std::optional, but that requires a lot of non-trivial out-of-scope work.

For now, I would use static_cast if possible or try to use Less() from SquidMath.h otherwise. In that context, we know for sure that dirno is not negative, of course.

Disclaimer: I have not reviewed many changes in this PR.

juju4 · 2024-10-31T19:09:54Z

yes, it's following the rabbit hole of changing #1118 (comment) and after trying to compile in the context of the debian package rebuild and with openssl as used in ansible role https://github.com/juju4/ansible-squid/
In my above context and only for ubuntu-24.04, build always fails if not patching.

On Must(swap_dirn < Config.cacheSwap.n_configured);, would Must( (size_t)swap_dirn < Config.cacheSwap.n_configured); be fine as change? or want more Must(static_cast<int>(swap_dirn) < Config.cacheSwap.n_configured); as per https://en.cppreference.com/w/cpp/language/static_cast ?

rousskov

I have not reviewed the entire PR, but these requests may help you make progress.

src/DiskIO/DiskThreads/aiops.cc

src/fs/ufs/UFSSwapDir.cc

src/pconn.h

rousskov · 2024-10-31T19:17:36Z

yes, it's following the rabbit hole of changing #1118 (comment) and after trying to compile in the context of the debian package rebuild and with openssl as used in ansible role https://github.com/juju4/ansible-squid/ In my above context and only for ubuntu-24.04, build always fails if not patching.

Do you recall how pconn.h changes were dragged into the mix? Perhaps there was another build error?

On Must(swap_dirn < Config.cacheSwap.n_configured);, would Must( (size_t)swap_dirn < Config.cacheSwap.n_configured); be fine as change? or want more Must(static_cast<int>(swap_dirn) < Config.cacheSwap.n_configured); as per https://en.cppreference.com/w/cpp/language/static_cast ?

In C++ code, please use static_cast (and other named casts) if possible.

rousskov · 2024-10-31T19:49:39Z

On Must(swap_dirn < Config.cacheSwap.n_configured);, would Must( (size_t)swap_dirn < Config.cacheSwap.n_configured); be fine as change? or want more Must(static_cast<int>(swap_dirn) < Config.cacheSwap.n_configured); as per https://en.cppreference.com/w/cpp/language/static_cast ?

In C++ code, please use static_cast (and other named casts) if possible.

... but with size_t rather than int type argument in this context, I presume.

juju4 · 2024-10-31T22:08:58Z

Current change results in compilation ending successfully.
basic local tests and ansible role CI tests: ok
https://github.com/juju4/ansible-squid/actions/runs/11620001325/job/32360862172

juju4 · 2024-11-02T16:02:50Z

Reverted all pconn.* changes. not compiling

libtool: link: rm -f ".libs/squid.nmI"
libtool: link: (cd .libs && gcc -g -O2 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -ffile-prefix-map=/var/cache/build/squid/squid-6.12=. -flto=auto -ffat-lto-objects -fstack-protector-strong -fstack-clash-protection -Wformat -Werror=format-security -fcf-protection -fdebug-prefix-map=/var/cache/build/squid/squid-6.12=/usr/src/squid-6.12-1 -Wno-error=deprecated-declarations -c -fno-builtin "squidS.c")
libtool: link: rm -f ".libs/squidS.c" ".libs/squid.nm" ".libs/squid.nmS" ".libs/squid.nmT" ".libs/squid.nmI"
libtool: link: g++ -std=c++17 -Wall -Wextra -Wimplicit-fallthrough=5 -Wpointer-arith -Wwrite-strings -Wcomments -Wshadow -Wmissing-declarations -Woverloaded-virtual -Werror -pipe -D_REENTRANT -I/usr/include/p11-kit-1 -g -O2 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -ffile-prefix-map=/var/cache/build/squid/squid-6.12=. -flto=auto -ffat-lto-objects -fstack-protector-strong -fstack-clash-protection -Wformat -Werror=format-security -fcf-protection -fdebug-prefix-map=/var/cache/build/squid/squid-6.12=/usr/src/squid-6.12-1 -Wno-error=deprecated-declarations .libs/squidS.o -Wl,-Bsymbolic-functions -flto=auto -ffat-lto-objects -Wl,-z -Wl,relro -Wl,-z -Wl,now -o squid AclRegs.o AuthReg.o BandwidthBucket.o delay_pools.o DelayId.o DelayBucket.o DelayConfig.o DelayPool.o DelaySpec.o DelayTagged.o DelayUser.o DelayVector.o MessageBucket.o MessageDelayPools.o ClientDelayConfig.o dns_internal.o htcp.o ipc.o snmp_core.o snmp_agent.o unlinkd.o AccessLogEntry.o AsyncEngine.o BodyPipe.o CacheDigest.o CachePeer.o CollapsedForwarding.o CommandLine.o ConfigOption.o ConfigParser.o CpuAffinity.o CpuAffinityMap.o CpuAffinitySet.o Downloader.o ETag.o EventLoop.o ExternalACLEntry.o FadingCounter.o FwdState.o HappyConnOpener.o HttpBody.o HttpControlMsg.o HttpHdrCc.o HttpHdrContRange.o HttpHdrRange.o HttpHdrSc.o HttpHdrScTarget.o HttpHeader.o HttpHeaderTools.o HttpReply.o HttpRequest.o HttpUpgradeProtocolAccess.o Instance.o LogTags.o MasterXaction.o MemBuf.o MemObject.o MemStore.o Notes.o Parsing.o PeerPoolMgr.o Pipeline.o RemovalPolicy.o RequestFlags.o ResolvedPeers.o SBufStatsAction.o SquidMath.o StatCounters.o StatHist.o StoreFileSystem.o StoreIOState.o StoreStats.o StoreSwapLogData.o StrList.o String.o Transients.o XactionInitiator.o cache_cf.o cache_manager.o carp.o cbdata.o clientStream.o client_db.o client_side.o client_side_reply.o client_side_request.o dlink.o errorpage.o event.o external_acl.o fatal.o fd.o fde.o filemap.o fqdncache.o fs_io.o helper.o http.o icp_v2.o icp_v3.o int.o internal.o ipcache.o main.o mem_node.o mime.o mime_header.o multicast.o neighbors.o pconn.o peer_digest.o peer_proxy_negotiate_auth.o peer_select.o peer_sourcehash.o peer_userhash.o redirect.o refresh.o stat.o stmem.o store.o store_client.o store_digest.o store_io.o store_key_md5.o store_log.o store_rebuild.o store_swapin.o store_swapout.o tools.o tunnel.o urn.o wccp.o wccp2.o whois.o wordlist.o LoadableModule.o LoadableModules.o globals.o hier_code.o icp_opcode.o lookup_t.o repl_modules.o swap_log_op.o -Wl,--export-dynamic  auth/.libs/libacls.a ident/.libs/libident.a acl/.libs/libacls.a acl/.libs/libstate.a auth/.libs/libauth.a acl/.libs/libapi.a clients/.libs/libclients.a servers/.libs/libservers.a ftp/.libs/libftp.a helper/.libs/libhelper.a http/.libs/libhttp.a dns/.libs/libdns.a base/.libs/libbase.a ./.libs/libsquid.a ip/.libs/libip.a fs/.libs/libfs.a DiskIO/.libs/libdiskio.a -lrt -lpthread comm/.libs/libcomm.a anyp/.libs/libanyp.a security/.libs/libsecurity.a ssl/.libs/libsslsquid.a ssl/.libs/libsslutil.a error/.libs/liberror.a ipc/.libs/libipc.a mgr/.libs/libmgr.a proxyp/.libs/libproxyp.a parser/.libs/libparser.a eui/.libs/libeui.a icmp/.libs/libicmp.a log/.libs/liblog.a format/.libs/libformat.a sbuf/.libs/libsbuf.a debug/.libs/libdebug.a repl/liblru.a repl/libheap.a -lcrypt adaptation/.libs/libadaptation.a -lecap snmp/.libs/libsnmp.a ../lib/snmplib/.libs/libsnmplib.a mem/.libs/libmem.a store/.libs/libstore.a time/.libs/libtime.a ../lib/.libs/libmisccontainers.a ../lib/.libs/libmiscencoding.a ../lib/.libs/libmiscutil.a -L/usr/lib64 -lcap -lssl -lcrypto -lgnutls -L/usr/lib/x86_64-linux-gnu/mit-krb5 -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lsystemd ../compat/.libs/libcompatsquid.a -lnettle -lm -lnetfilter_conntrack -ldl -L.. -lltdl
pconn.cc: In member function '__ct_base ':
pconn.cc:43:53: error: argument 1 value '18446744073709551615' exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
   43 |     theList_ = new Comm::ConnectionPointer[capacity_];
      |                                                     ^
/usr/include/c++/13/new:128:26: note: in a call to allocation function 'operator new []' declared here
  128 | _GLIBCXX_NODISCARD void* operator new[](std::size_t) _GLIBCXX_THROW (std::bad_alloc)
      |                          ^
pconn.cc: In member function 'push':
pconn.cc:174:57: error: argument 1 value '18446744073709551615' exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
  174 |         theList_ = new Comm::ConnectionPointer[capacity_];
      |                                                         ^
/usr/include/c++/13/new:128:26: note: in a call to allocation function 'operator new []' declared here
  128 | _GLIBCXX_NODISCARD void* operator new[](std::size_t) _GLIBCXX_THROW (std::bad_alloc)
      |                          ^
lto1: all warnings being treated as errors
make[4]: *** [/tmp/ccdo9Aih.mk:104: /tmp/ccVHICKP.ltrans34.ltrans.o] Error 1
make[4]: *** Waiting for unfinished jobs....
lto-wrapper: fatal error: make returned 2 exit status
compilation terminated.
/usr/bin/ld: error: lto-wrapper failed
collect2: error: ld returned 1 exit status
libtool: link: rm -f ".libs/squidS.o"
make[3]: *** [Makefile:5255: squid] Error 1
make[3]: Leaving directory '/var/cache/build/squid/squid-6.12/src'

juju4 · 2024-11-03T23:33:01Z

I think that @yadij work in 2d232fa#diff-44d0ed5297324f87595e0aad7e946626d844235cbb38a0d02efac9bd1741f795 covers this. If yes, can be closed. Please confirm @rousskov

rousskov · 2024-11-04T03:16:34Z

Reverted all pconn.* changes. not compiling

pconn.cc:43:53: error: argument 1 value '18446744073709551615' exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
43 |     theList_ = new Comm::ConnectionPointer[capacity_];

Thank you. We now know that the problem affects more than just store/Disks.cc code. I was not aware of that fact before. We will need to fix pconn code after all, despite extra problems such a fix will need to address. I was hoping to avoid that extra work, but that is not an option AFAICT.

I think that @yadij work in 2d232fa#diff-44d0ed5297324f87595e0aad7e946626d844235cbb38a0d02efac9bd1741f795 covers this. If yes, can be closed. Please confirm @rousskov

Sorry, I do not understand what you would like me to confirm. I would rather not review another PR in this PR, but commit 2d232fa in #1118 does not appear to address the problem you faced when modifying pconn code in this PR, does it?

FWIW, I do not know why Amos committed to old #1118 when you were making good progress with this PR #1929. Hopefully, he clarifies, and we would not have to work on two very similar PRs! @yadij, do you want to close this recently (partially) reviewed PR and resume pushing forward old #1118 instead?

yadij · 2024-11-05T10:39:35Z

@yadij, do you want to close this recently (partially) reviewed PR and resume pushing forward old #1118 instead?

If @juju4 is okay with that. Up to you.

Notes:

My addition to Bug 5160: Compile fails with -flto=auto [-Walloc-size-larger-than=] #1118 were already done for the Debian package, but not yet merged there or submitted to us.
It (Bug 5160: Compile fails with -flto=auto [-Walloc-size-larger-than=] #1118) builds fully for me (GCC v14.2.0) using ./configure CXXFLAGS="-flto=auto" && make check but has not had run-time testing.

rousskov · 2024-11-05T14:08:45Z

@yadij, do you want to close this recently (partially) reviewed PR and resume pushing forward old #1118 instead?

If @juju4 is okay with that. Up to you.

If it were up to me, then this recently (partially) reviewed PR backed by an active first-time contributor would continue to be advanced¹, but I do not insist on any strategy. @juju4, what is your preference?

My addition to Bug 5160: Compile fails with -flto=auto [-Walloc-size-larger-than=] #1118 were already done for the Debian package, but not yet merged there or submitted to us.

If you refer to commit 2d232fa, then I hope that it is not used by anybody until my concern is addressed.

Instead of advancing old PR 1118 that should not have been suddenly updated when the work has been progressing nicely here, especially when that update disregarded partial results in this PR! ↩

juju4 · 2024-11-06T23:54:08Z

I'm fine either way, whatever get issue fixed and well.

juju4 · 2024-11-24T20:46:31Z

any plan on this?

rousskov · 2024-11-25T01:18:39Z

any plan on this?

Since nobody seems to be driving this, I plan to use this PR branch to address pconn.cc breakage without triggering the earlier problem and then ask you to retest. This plan may require a prerequisite PR for pconn improvements, but it is too early for me to say. I hope to start implementing this plan within a week or two.

yadij · 2024-12-03T05:28:04Z

Since nobody seems to be driving this, I plan to use this PR branch ...

In that case, can you finalize the checks on PR #1118 and mark it for merge if it passes.

rousskov · 2024-12-03T13:50:12Z

Alex: Since nobody seems to be driving this, I plan to use this PR branch ...

Amos: In that case, can you finalize the checks on PR #1118 and mark it for merge if it passes.

What do you mean by "finalize the checks"? Both PRs require significant code changes before either can be merged, as I mentioned earlier. I also mentioned why I prefer to continue to work with this PR. If you would like to change my plan to work with this PR despite saying that it is up to me, please explain why that plan should be changed.

This reduction is not just a cosmetic improvement. It increases the chance of discovering stale/mismatching printf() formats and similar problems when reviewing the diff of changes (because it places the modified line much closer to actual index use cases inside the loop), improving our overall confidence in these changes. Also upgraded Disks::store() and Disks::Dir() argument to size_t because these methods use SwapDirByIndex() that was upgraded in an earlier branch commit. I checked that the removed variants are no longer used before removing those variants (by making Squid with those methods explicitly deleted).

It is derived from upgraded firstCandidate. It is used as an argument for upgraded SwapDirByIndex().

They were necessary (LTO builds fails without them AFAICT), and they themselves were not wrong (just insufficient).

... because we are not upgrading its source -- Store::Disk::index (yet). This change also fixes "make check".

... because we are not upgrading its sources -- Store::Disk::index and UFSDirToGlobalDirMapping[i] (yet).

rousskov · 2024-12-07T03:17:04Z

Alex: I plan to use this PR branch to address pconn.cc breakage without triggering the earlier problem

Done (as of branch commit b21738e). I also committed a few polishing changes.

Unfortunately, my attempts to reproduce the primary problem with GCC v12 on Ubuntu 22.04 (by adding -flto=auto -ffat-lto-objects or many flags from https://github.com/juju4/ansible-squid/actions/runs/11620001325/job/32360862172 logs to CXXFLAGS) have failed. Thus, I was operating in the dark and could have missed something important. If you know how I can reproduce those -Walloc-size-larger-than= warnings in that environment, please let me know!

@juju4, please review my changes and, most importantly, retest. Thank you.

P.S. I will help fixing PR description later, if needed, once we are happy with code changes.

My concerns were addressed.

rousskov · 2024-12-07T03:59:04Z

Alex: Thank you for improving so many declarations! Please add your contact info to CONTRIBUTORS file.

@juju4, the above request still stands. Our CI cannot tell which variation of that info should be used, but once you add one of the two variants to CONTRIBUTORS, that check should succeed. If you want to add a completely different name and email pair, consider changing your GitHub account setup -- GitHub gets one of those variants from your account information IIRC. If you would like to add nothing (i.e. keep CONTRIBUTORS intact), we should be able to accommodate that as well (but it will probably require some manual action to work around that CI check limitations, and your GitHub contact info will still be a part of the official commit metadata).

juju4 · 2024-12-08T21:02:19Z

@juju4, please review my changes and, most importantly, retest. Thank you.
Thanks a lot!
Will try to review next week-end.

juju4 · 2024-12-15T22:33:11Z

Not much time in the end and got stopped in configure...
https://github.com/juju4/ansible-squid/actions/runs/12342605476/job/34442632278#step:8:691

"checking for dlopen in -ldl... no", "checking whether linking without -latomic works... no", "checking whether linking with -latomic works... no"

what is the matching package?
libatomic1 is installed and there is no corresponding -dev package. redhat family has -devel package but not debian/ubuntu.

Note: it seems the basic configure without arguments does not need it and can build fine. but as mirroring debian package arguments, much more.

rousskov · 2024-12-16T01:06:32Z

Not much time in the end and got stopped in configure... https://github.com/juju4/ansible-squid/actions/runs/12342605476/job/34442632278#step:8:691

Since this PR does not change anything related to ./configure, that problem feels out of scope. Did ./configure step work before this PR changes? If yes, what has changed (other than changes introduced by this PR)? Otherwise, perhaps we can go back to your tests that were failing when you posted this PR (those tests failed after ./configure, not during ./configure)?

"checking for dlopen in -ldl... no", 
"checking whether linking without -latomic works... no",
"checking whether linking with -latomic works... no"

what is the matching package? libatomic1 is installed and there is no corresponding -dev package. redhat family has -devel package but not debian/ubuntu.

I do not know enough about your build environment to answer your question. I see much earlier scary warnings, but I do not know whether they are relevant:

WARNING: invalid host type

Note: it seems the basic configure without arguments does not need it and can build fine. but as mirroring debian package arguments, much more.

Perhaps it is possible to make progress by not mirroring any external/independent package arguments (at least for now)?

The build log you referenced also talks about squid-6.12.tar.gz. Is it actually building this PR code??

juju4 · 2024-12-16T02:03:17Z

The build log you referenced also talks about squid-6.12.tar.gz. Is it actually building this PR code??

Not for now. I first need an environment that build and that have some testing. My ansible role has that for debian package upstream or the rebuild deb package with openssl for which I started to dig this. I'm extending to source build before adding and validating it works before adding patch either against 6.12 or latest HEAD. the deb package rebuild works.
Build environment is a fresh ubuntu 24.04 (lxc container) on my side or ubuntu various (azure) for github actions (notably customized as per runner images).

rousskov · 2024-12-16T02:56:03Z

Unfortunately, I am not familiar with half of the terms you are using, and I still do not understand why you cannot repeat the tests that initiated this PR (and that failed during make rather than ./configure) but do that using PR code. Nothing else needs to change AFAICT.

FWIW, the current goal is (or should be) to validate whether PR changes are sufficient to fix LTO build. Since I cannot reproduce LTO build failures in my build environment, I cannot perform that validation myself. If you can focus on that goal, please do so. Otherwise, I hope you will figure out how to fix those (seemingly unrelated) problems and then get to testing this PR changes. I am happy to help, but I do not know much about ansible, "ansible role", "debian package upstream", "rebuild deb package", "extending to source build" and related customizations/options/flags...

juju4 · 2024-12-22T22:13:47Z

After more manual review

per config.log, libatomic issue comes from a multiline "#define SQUID_CONFIGURE_OPTIONS" which comes from using ansible module command and not shell, losing the return line escape but not completed correctly. fixed.
compilation of 6.12 source on Ubuntu 22.04, 24.04 and Debian 12, ok except for ecap which I had to remove ("configure: error: eCAP support requires loadable modules. Please do not use --disable-shared with --enable-ecap." but no "--disabled-shared" in my configure arguments...). example: https://github.com/juju4/ansible-squid/actions/runs/12448936189/job/34754042869#step:8:1520
adding usual debian build FLAGS leads back to build error on ubuntu-24.04 and debian-12 that this PR is targeting. https://github.com/juju4/ansible-squid/actions/runs/12449071671/job/34754314159#step:8:737
adding PR patch leads to build success and other role tests too, including "make check" and normal squid usage (query http, https, blocked acl).
Patch addresses correctly LTO build for Ubuntu-24.04. https://github.com/juju4/ansible-squid/actions/runs/12449208676/job/34754593489#step:8:1522
Debian-12 has a regression and failed on different error. https://github.com/juju4/ansible-squid/actions/runs/12449208676/job/34754593625#step:8:732

/usr/bin/ar: `u' modifier ignored since `D' is the default (see `U')
/usr/bin/ar: `u' modifier ignored since `D' is the default (see `U')
/usr/bin/ar: `u' modifier ignored since `D' is the default (see `U')
ipc/MemMap.cc: In function 'store_session_cb':
ipc/MemMap.cc:302:11: error: writing 32 bytes into a region of size 0 [-Werror=stringop-overflow=]
  302 |     memcpy(key, aKey, sizeof(key));
      |           ^\nsecurity/../../src/ipc/mem/FlexibleArray.h:38:29: note: at offset [20, 21] into destination object 'start_' of size 1
   38 |     alignas(Item) std::byte start_; ///< the first byte of the first array item
      |                             ^\nlto1: all warnings being treated as errors
make[4]: *** [/tmp/ccsrf9t0.mk:215: /tmp/ccgsmphI.ltrans71.ltrans.o] Error 1
make[4]: *** Waiting for unfinished jobs....
lto-wrapper: fatal error: make returned 2 exit status
compilation terminated.
/usr/bin/ld: error: lto-wrapper failed
collect2: error: ld returned 1 exit status
make[3]: *** [Makefile:5255: squid] Error 1

Is there anything more that you want me to test?

rousskov · 2024-12-23T03:28:33Z

compilation of 6.12 source on Ubuntu 22.04, 24.04 and Debian 12, ok except for ecap which I had to remove ("configure: error: eCAP support requires loadable modules. Please do not use --disable-shared with --enable-ecap." but no "--disabled-shared" in my configure arguments...). example: https://github.com/juju4/ansible-squid/actions/runs/12448936189/job/34754042869#step:8:1520

It looks like after 2022 commit 5a2409b, that "do not use" advice became misleading in rare cases where shared library support is disabled for reasons other than an explicit --disable-shared argument. I tried to polish that in #1968.

adding usual debian build FLAGS leads back to build error on ubuntu-24.04 and debian-12 that this PR is targeting. https://github.com/juju4/ansible-squid/actions/runs/12449071671/job/34754314159#step:8:737

Glad you were able to reproduce the error this PR is targeting.

adding PR patch leads to build success and other role tests too

Great. AFAICT, that result confirms that this PR accomplishes its primary goal. Please correct me if I am wrong.

To move this PR forward, I adjusted PR title to use GCC v13 because that is what you are testing with (AFAICT) and removed temporary comments from PR description (i.e. the future official commit message if this PR is merged). I am also marking this PR as ready for review.

Debian-12 has a regression and failed on different error. https://github.com/juju4/ansible-squid/actions/runs/12449208676/job/34754593625#step:8:732

ipc/MemMap.cc: In function 'store_session_cb':
ipc/MemMap.cc:302:11: error: writing 32 bytes into a region of size 0 [-Werror=stringop-overflow=]
  302 |     memcpy(key, aKey, sizeof(key));
      |           ^\nsecurity/../../src/ipc/mem/FlexibleArray.h:38:29: note: at offset [20, 21] into destination object 'start_' of size 1
   38 |     alignas(Item) std::byte start_; ///< the first byte of the first array item

This problem is best addressed by another PR. Right now, I do not know how to address that problem. Filing a bug report with Squid Bugzilla may the right next step. If you do that, please mention whether the problem exists only in LTO-enabled builds.

Is there anything more that you want me to test?

I cannot think of anything but please fix this PR CONTRIBUTORS file as discussed earlier.

fix: GCC v12 build [-Walloc-size-larger-than=] - type review

3fd91ea

rousskov marked this pull request as draft October 31, 2024 18:13

rousskov changed the title ~~fix: GCC v12 build [-Walloc-size-larger-than=] - type review~~ Fix GCC v12 build [-Walloc-size-larger-than=] Oct 31, 2024

rousskov added the S-waiting-for-author author action is expected (and usually required) label Oct 31, 2024

rousskov previously requested changes Oct 31, 2024

View reviewed changes

src/DiskIO/DiskThreads/aiops.cc Outdated Show resolved Hide resolved

src/fs/ufs/UFSSwapDir.cc Show resolved Hide resolved

src/pconn.h Show resolved Hide resolved

fix: GCC v12 build [-Walloc-size-larger-than=] - PR follow-up

e1a2377

fix: reverting most pconn.* changes

74b9ba3

fix: reverting last pconn.* changes

87dc48d

Merge branch 'master' into devel-gcc-fix

e02c85b

rousskov mentioned this pull request Nov 4, 2024

Bug 5160: Compile fails with -flto=auto [-Walloc-size-larger-than=] #1118

Open

rousskov added 7 commits December 6, 2024 20:57

fixup: upgrade dirn as well

06d9d95

It is derived from upgraded firstCandidate. It is used as an argument for upgraded SwapDirByIndex().

Restored pconn.* changes

d6ce139

They were necessary (LTO builds fails without them AFAICT), and they themselves were not wrong (just insufficient).

fixup: Avoid subtracting from zero IdleConnList::size_

e4573ac

fixup: Do not cast size_t to size_t

a3cb668

fixup: const-correctness and out-of-scope TODO

bffc9fe

Do not upgrade storeRebuildProgress index parameter

9878481

... because we are not upgrading its source -- Store::Disk::index (yet). This change also fixes "make check".

Do not upgrade UFSSwapDir::FilenoBelongsHere F0 parameter

b21738e

... because we are not upgrading its sources -- Store::Disk::index and UFSDirToGlobalDirMapping[i] (yet).

rousskov changed the title ~~Fix GCC v12 build [-Walloc-size-larger-than=]~~ Fix GCC v12 LTO build [-Walloc-size-larger-than=] Dec 7, 2024

fixup: Fix build broken by my previous branch commit

042cd22

rousskov changed the title ~~Fix GCC v12 LTO build [-Walloc-size-larger-than=]~~ Fix GCC v13 LTO build [-Walloc-size-larger-than=] Dec 23, 2024

rousskov marked this pull request as ready for review December 23, 2024 03:28

rousskov mentioned this pull request Dec 23, 2024

Clarify --enable-ecap failure on missing shared library support #1968

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix GCC v13 LTO build [-Walloc-size-larger-than=] #1929

Fix GCC v13 LTO build [-Walloc-size-larger-than=] #1929

juju4 commented Oct 31, 2024 •

edited by rousskov

Loading

rousskov commented Oct 31, 2024

juju4 commented Oct 31, 2024 •

edited

Loading

rousskov left a comment

rousskov commented Oct 31, 2024

rousskov commented Oct 31, 2024

juju4 commented Oct 31, 2024

juju4 commented Nov 2, 2024

juju4 commented Nov 3, 2024

rousskov commented Nov 4, 2024

yadij commented Nov 5, 2024

rousskov commented Nov 5, 2024

juju4 commented Nov 6, 2024

juju4 commented Nov 24, 2024

rousskov commented Nov 25, 2024

yadij commented Dec 3, 2024

rousskov commented Dec 3, 2024

rousskov commented Dec 7, 2024 •

edited

Loading

rousskov commented Dec 7, 2024

juju4 commented Dec 8, 2024

juju4 commented Dec 15, 2024

rousskov commented Dec 16, 2024

juju4 commented Dec 16, 2024

rousskov commented Dec 16, 2024

juju4 commented Dec 22, 2024

rousskov commented Dec 23, 2024

Fix GCC v13 LTO build [-Walloc-size-larger-than=] #1929

Are you sure you want to change the base?

Fix GCC v13 LTO build [-Walloc-size-larger-than=] #1929

Conversation

juju4 commented Oct 31, 2024 • edited by rousskov Loading

rousskov commented Oct 31, 2024

juju4 commented Oct 31, 2024 • edited Loading

rousskov left a comment

Choose a reason for hiding this comment

rousskov commented Oct 31, 2024

rousskov commented Oct 31, 2024

juju4 commented Oct 31, 2024

juju4 commented Nov 2, 2024

juju4 commented Nov 3, 2024

rousskov commented Nov 4, 2024

yadij commented Nov 5, 2024

rousskov commented Nov 5, 2024

Footnotes

juju4 commented Nov 6, 2024

juju4 commented Nov 24, 2024

rousskov commented Nov 25, 2024

yadij commented Dec 3, 2024

rousskov commented Dec 3, 2024

rousskov commented Dec 7, 2024 • edited Loading

rousskov commented Dec 7, 2024

juju4 commented Dec 8, 2024

juju4 commented Dec 15, 2024

rousskov commented Dec 16, 2024

juju4 commented Dec 16, 2024

rousskov commented Dec 16, 2024

juju4 commented Dec 22, 2024

rousskov commented Dec 23, 2024

juju4 commented Oct 31, 2024 •

edited by rousskov

Loading

juju4 commented Oct 31, 2024 •

edited

Loading

rousskov commented Dec 7, 2024 •

edited

Loading