Bug #10469
closedGCC's -faggressive-loop-optimizations is too aggressive
100%
Description
Based on experience in OS-7511 with GCC optimizations eliding undefined but de facto correct code, a full build was performed with -fno-aggressive-loop-optimizations
and then functions were compared to determine changes in text size. First, it should be said that most functions were not affected at all; of the 782027 functions, there were text size differences in only 2286 of them. Of these, text size increased in about half of them (1026). In investigating these, they seem to primarily be due to different register selection, often because the presence of the optimization seems to cause loop counters (and address calculation) to be replaced with addresses and address bounds.
Here are the functions for which -faggressive-loop-optimizations
resulted in less program text and fewer backwards branches, ordered by fraction of text eliminated:
#backwards branches, -faggressive-loop-optimizations ------------------------+ #backwards branches, -fno-aggressive-loop-optimizations ----------------+ | #instructions, -faggressive-loop-optimizations --------------------+ | | #instructions, -fno-aggressive-loop-optimizations ----------+ | | | | | | | V V V V MOD`FUNC #INST O#INST #BB O#BB usr/lib/libpool.so.1`pool_resource_type_list 116 66 2 0 kernel/drv/amd64/xge`xgell_tx_close 122 71 2 0 platform/i86pc/kernel/drv/amd64/dr`dr_bypass_device 82 50 1 0 usr/lib/libadutils.so.1`check_for_binary_attrs 80 51 1 0 kernel/drv/amd64/qede`qede_free_tx_bd_ring 122 80 1 0 usr/sbin/i86/zdb`get_metaslab_refcount 223 147 7 6 kernel/drv/amd64/qede`qede_alloc_io_structs 166 112 2 1 usr/lib/amd64/libstandsaveargs.so`has_saved_fp 220 151 4 2 usr/lib/amd64/libsaveargs.so.1`has_saved_fp 220 151 4 2 kernel/misc/amd64/kmdbmod`has_saved_fp 220 151 4 2 kernel/drv/amd64/qede`ecore_int_disable_post_isr_release 67 46 1 0 usr/kernel/fs/amd64/lx_proc`lxpr_skip_mntopt 91 63 1 0 usr/lib/libzpool.so.1`vdev_root_core_tvds 172 121 5 4 usr/lib/amd64/gss/mech_krb5.so.1`rem_com_err_hook 187 135 4 3 usr/lib/smbsrv/amd64/libfksmbsrv.so.1`smb2_supported_version 95 71 1 0 usr/lib/amd64/libvarpd.so.1`libvarpd_prop_set 194 146 2 1 usr/lib/ndmp/ndmpd`ndmpd_make_exc_list 78 59 1 0 kernel/fs/amd64/zfs`dnode_needs_remap 252 191 3 1 kernel/drv/amd64/smrt`smrt_interrupts_free 101 77 1 0 usr/lib/libzpool.so.1`dnode_needs_remap 325 250 4 3 usr/lib/libzpool.so.1`vdev_count_leaves_impl 101 79 3 2 kernel/fs/amd64/zfs`dmu_objset_write_ready 175 137 1 0 usr/lib/amd64/libzpool.so.1`dnode_needs_remap 309 245 4 2 usr/lib/amd64/libadutils.so.1`check_for_binary_attrs 114 92 1 0 kernel/drv/amd64/qede`qede_fastpath_config 447 364 2 1 usr/kernel/drv/amd64/smbsrv`smb2_supported_version 87 71 1 0 usr/lib/libzpool.so.1`dmu_object_info_from_dnode 405 339 3 1 kernel/drv/amd64/emlxs`emlxs_fcoe_fcftab_fcfi_online_action 618 518 5 3 usr/sbin/i86/zdb`get_dtl_refcount 140 118 3 2 usr/lib/amd64/libzfs_jni.so.1`zjni_get_default_property 283 241 4 3 usr/lib/mdb/kvm/amd64/fcp.so`targets_walk_i 212 182 4 3 kernel/kmdb/amd64/fcp`targets_walk_i 212 182 4 3 kernel/drv/amd64/qede`qede_init_locks 341 293 2 1 kernel/drv/amd64/bnxe`elink_avoid_link_flap 551 474 7 6 kernel/fs/amd64/zfs`dmu_object_info_from_dnode 323 280 2 1 usr/xpg6/bin/view`dhl_doit 242 210 4 2 usr/xpg6/bin/vi`dhl_doit 242 210 4 2 usr/xpg6/bin/vedit`dhl_doit 242 210 4 2 usr/xpg6/bin/expr`dhl_doit 242 210 4 2 usr/xpg6/bin/ex`dhl_doit 242 210 4 2 usr/xpg6/bin/edit`dhl_doit 242 210 4 2 usr/xpg6/bin/ed`dhl_doit 242 210 4 2 usr/xpg4/bin/view`dhl_doit 242 210 4 2 usr/xpg4/bin/vi`dhl_doit 242 210 4 2 usr/xpg4/bin/vedit`dhl_doit 242 210 4 2 usr/xpg4/bin/nl`dhl_doit 242 210 4 2 usr/xpg4/bin/expr`dhl_doit 242 210 4 2 usr/xpg4/bin/ex`dhl_doit 242 210 4 2 usr/xpg4/bin/edit`dhl_doit 242 210 4 2 usr/xpg4/bin/ed`dhl_doit 242 210 4 2 usr/bin/red`dhl_doit 242 210 4 2 usr/bin/nl`dhl_doit 242 210 4 2 usr/bin/expr`dhl_doit 242 210 4 2 usr/bin/ed`dhl_doit 242 210 4 2 kernel/drv/amd64/emlxs`emlxs_fcoe_fcftab_offline_timer 186 162 1 0 usr/lib/libzpool.so.1`dmu_objset_write_ready 397 346 5 3 lib/amd64/libnsl.so.1`getflag 141 123 4 3 lib/amd64/libadm.so.1`puttext 1833 1601 35 31 usr/sbin/iscsiadm`usage 270 237 7 4 usr/lib/amd64/libzpool.so.1`dmu_object_info_from_dnode 355 312 2 1 lib/libdhcputil.so.1`category_to_code 89 79 2 1 lib/amd64/libcurses.so.1`waddwchnstr 721 641 12 9 kernel/drv/amd64/nxge`nxge_update_rxdma_grp_properties 1440 1284 28 27 kernel/drv/amd64/nxge`nxge_cfg_verify_set 655 585 17 15 kernel/drv/amd64/qede`qede_destroy_locks 200 179 2 1 lib/libc.so.1`door_xcreate_n 1271 1139 26 20 kernel/drv/amd64/qede`qede_fastpath_start_queues 824 739 6 3 usr/lib/libc/libc_hwcap3.so.1`door_xcreate_n 1245 1121 23 20 usr/lib/libc/libc_hwcap2.so.1`door_xcreate_n 1245 1121 23 20 usr/lib/libc/libc_hwcap1.so.1`door_xcreate_n 1245 1121 23 20 usr/lib/amd64/gss/mech_krb5.so.1`krb5_c_keyed_checksum_types 338 305 7 3 usr/lib/security/amd64/pkcs11_kernel.so.1`kernel_get_func_list 430 390 6 4 kernel/drv/amd64/mc-amd`iaddr_to_bank 188 171 2 1 usr/lib/amd64/libzpool.so.1`dmu_objset_write_ready 354 322 3 2 lib/libnvpair.so.1`nvt_resize 222 202 4 3 lib/libcurses.so.1`waddwchnstr 713 649 18 16 kernel/drv/amd64/emlxs`emlxs_fcoe_fcftab_fcfi_offline_action 456 416 3 2 platform/i86xpv/kernel/amd64/unix`page_ctrs_cleanup 195 178 2 1 platform/i86pc/kernel/amd64/unix`page_ctrs_cleanup 195 178 2 1 usr/sbin/i86/zdb`dump_bpobj_subobjs 355 325 5 4 kernel/drv/amd64/qede`ecore_hw_timers_stop_all 98 90 1 0 lib/libnsl.so.1`getflag 89 82 5 2 usr/lib/amd64/gss/mech_krb5.so.1`set_com_err_hook 118 109 1 0 kernel/drv/amd64/emlxs`emlxs_parse_prog_types 1633 1509 47 46 lib/amd64/libdhcputil.so.1`category_to_code 108 100 2 1 lib/amd64/libzfs.so.1`zfeature_register 222 206 2 1 usr/lib/krb5/libkadm5srv.so.1`kadm5_free_key_data 127 118 2 1 usr/lib/krb5/libkadm5clnt.so.1`kadm5_free_key_data 127 118 2 1 kernel/drv/amd64/ixgbe`ixgbe_chip_start 345 321 5 4 kernel/drv/amd64/qede`qede_kstat_update 1078 1004 4 3 kernel/fs/amd64/zfs`zfeature_register 238 222 2 1 usr/kernel/fs/amd64/lx_proc`lxpr_open 286 267 5 3 kernel/drv/amd64/xge`xgell_tx_open 109 102 1 0 kernel/fs/amd64/zfs`vdev_raidz_reconstruct 344 322 14 13 kernel/drv/amd64/nxge`nxge_update_txdma_properties 1478 1385 27 24 kernel/drv/amd64/nxge`nxge_update_rxdma_properties 1478 1385 27 24 kernel/drv/amd64/qede`qede_save_fp_dma_handles 479 449 4 3 kernel/drv/amd64/qede`qede_free_tx_ring_phys 130 122 1 0 usr/bin/amd64/nm`readsyms 776 729 15 14 kernel/strmod/amd64/ip`rule_prefix 117 110 1 0 kernel/drv/amd64/ip`rule_prefix 117 110 1 0 usr/bin/geniconvtbl`map_table_hash 1720 1619 39 38 kernel/fs/amd64/udfs`ud_read_sparing_tbls 327 308 4 2 usr/sbin/isns`setup_ref_lcp 437 412 7 5 platform/i86xpv/kernel/amd64/unix`page_ctrs_sz 581 548 9 8 platform/i86pc/kernel/amd64/unix`page_ctrs_sz 581 548 9 8 usr/bin/ftp`mypopen 480 453 11 9 usr/lib/libzpool.so.1`vdev_compact_children 267 252 6 4 usr/lib/amd64/libtecla.so.1`gl_start_newline 220 208 3 2 lib/amd64/libnsl.so.1`expect_str 303 287 3 1 usr/kernel/drv/amd64/ipf`ippr_rpcb_modv4 537 509 4 2 kernel/misc/amd64/usba`usb_pipe_isoc_xfer 422 400 6 5 usr/lib/amd64/gss/mech_krb5.so.1`reset_com_err_hook 117 111 1 0 usr/lib/gss/mech_krb5.so.1`krb5int_get_plugin_filenames 326 310 7 6 usr/lib/amd64/libldap.so.5`ldif_base64_encode_internal 502 478 12 11 usr/sbin/isnsadm`cvt_enumerate_rsp_to_get_req 1517 1445 31 30 usr/lib/amd64/libast.so.1`_ast_ftwalk 1105 1057 27 24 usr/lib/nss_ldap.so.1`add_netgroup_name 236 226 7 5 usr/lib/libsun_ima.so.1`isAuthMethodValid 98 94 2 1 kernel/misc/amd64/nfssrv`nfslog_write_record 735 706 8 5 usr/lib/libdtrace.so.1`dt_fprintas 128 123 3 2 usr/lib/ipf/amd64/ipftest`ippr_rpcb_modv4 449 432 5 3 lib/libresolv.so.2`res_nsend 2223 2139 32 31 usr/sbin/snoop`concat_args 213 205 6 5 kernel/drv/amd64/bnxe`BnxeHwReqPhyFlowSettings 493 475 5 4 usr/lib/libzpool.so.1`raidz_parity_verify 367 354 6 5 usr/sbin/ipqosconf`get_originator_nm 57 55 3 1 usr/ucb/groups`main 286 276 4 3 usr/bin/amd64/savecore`BZ2_hbMakeCodeLengths 1375 1327 26 25 kernel/amd64/genunix`BZ2_hbMakeCodeLengths 1375 1327 26 25 platform/i86xpv/kernel/drv/amd64/xsvc`xsvc_devmap 774 747 8 7 usr/lib/libzfs_jni.so.1`zjni_get_default_property 324 313 10 9 platform/i86pc/kernel/drv/amd64/xsvc`xsvc_devmap 750 725 8 7 usr/lib/amd64/libsldap.so.1`__ns_ldap_trydoorcall_getfd 496 480 8 6 usr/bin/rdist`docmdargs 409 396 8 7 usr/kernel/fs/amd64/lx_proc`lxpr_doaccess 519 503 14 13 lib/amd64/libc.so.1`__k_gconvert 801 777 20 18 kernel/drv/amd64/yge`yge_resume 541 525 7 6 platform/i86xpv/kernel/amd64/unix`hment_mapcnt 273 265 6 4 platform/i86pc/kernel/amd64/unix`hment_mapcnt 273 265 6 4 kernel/drv/amd64/qede`qede_fastpath_stop_queues 567 551 7 6 kernel/drv/amd64/si3124`si_watchdog_handler 538 523 10 8 usr/lib/amd64/gss/mech_krb5.so.1`prof_hostnames2netaddrs 582 566 13 12 kernel/drv/amd64/rpcib`rib_svc_scq_handler 329 320 5 4 kernel/drv/amd64/rpcib`rib_clnt_scq_handler 329 320 5 4 kernel/misc/amd64/usba`usb_pipe_intr_xfer 597 581 10 9 usr/sbin/iasl`LsFlushListingBuffer 608 592 9 8 kernel/drv/amd64/ural`ural_set_chan 1107 1078 11 10 kernel/misc/amd64/usba`usb_pipe_ctrl_xfer 618 602 11 9 usr/bin/amd64/pflags`lwplook 961 937 15 14 lib/amd64/libresolv.so.2`res_nsend 2119 2071 31 29 kernel/drv/amd64/t4nex`t4_devo_detach 819 801 9 8 usr/platform/i86pc/lib/fm/topo/plugins/x86pi.so`x86pi_bb_topparent 393 385 9 7 usr/bin/tail`lines 1033 1012 28 27 usr/sbin/isnsadm`cmdParse 1880 1842 34 32 usr/bin/lex`acompute 768 753 20 19 kernel/drv/amd64/qede`qede_alloc_tx_ring_phys 362 355 5 0 usr/bin/audiotest`uncompress_wave 1379 1353 15 14 kernel/drv/amd64/dcam1394`dcam_ioctl 850 834 19 17 usr/lib/amd64/libzpool.so.1`vdev_raidz_reconstruct 856 840 23 22 usr/bin/i86/ztest`make_vdev_root 482 473 7 6 kernel/misc/amd64/pcmcia`SSSetSocket 379 372 2 0 usr/sbin/stmfadm`cmdParse 2078 2040 34 32 usr/sbin/sbdadm`cmdParse 2078 2040 34 32 usr/sbin/sasinfo`cmdParse 2078 2040 34 32 usr/sbin/fcinfo`cmdParse 2078 2040 34 32 usr/sbin/fcadm`cmdParse 2078 2040 34 32 usr/demo/comstar/bin/aluaadm`cmdParse 2078 2040 34 32 lib/svc/method/svc-stmf`cmdParse 2078 2040 34 32 lib/svc/method/iscsi-target`cmdParse 2078 2040 34 32 usr/lib/amd64/libzpool.so.1`zfs_blkptr_verify 938 921 19 18 kernel/drv/amd64/bnxe`BnxeHwReqPhyMediumSettings 3087 3032 43 41 usr/lib/amd64/libsldap.so.1`sortServerPref 907 891 30 29 kernel/drv/amd64/yge`yge_detach 455 447 2 1 kernel/misc/amd64/usba`usb_pipe_bulk_xfer 467 459 10 8 usr/lib/libsldap.so.1`verify_value 733 721 18 10 usr/lib/libc/libc_hwcap3.so.1`glob0 1447 1424 66 62 usr/lib/libc/libc_hwcap3.so.1`glob0 1447 1424 66 62 usr/lib/libc/libc_hwcap2.so.1`glob0 1447 1424 66 62 usr/lib/libc/libc_hwcap2.so.1`glob0 1447 1424 66 62 usr/lib/libc/libc_hwcap1.so.1`glob0 1447 1424 66 62 usr/lib/libc/libc_hwcap1.so.1`glob0 1447 1424 66 62 usr/lib/ipf/i86/ipftest`ippr_rpcb_modv4 441 434 5 3 usr/sbin/amd64/lockstat`dprog_addevent 1116 1100 11 10 usr/lib/gss/mech_krb5.so.1`rem_com_err_hook 146 144 5 4 usr/bin/troff`casecs 146 144 3 1 usr/sbin/mpathadm`cmdParse 2585 2550 39 37 usr/bin/split`main 2954 2914 55 54 usr/sbin/iscsiadm`cmdParse 2623 2588 39 37 usr/lib/mdb/kvm/amd64/genunix.so`Pcred_gcore 309 305 3 2 kernel/drv/amd64/bnxe`ecore_config_vlan_mac 391 386 8 5 usr/sbin/ndmpadm`ndmp_kill_sessions 236 233 5 4 lib/amd64/libc.so.1`format_grouped_double 1287 1271 17 16 usr/bin/pg`main 1384 1367 29 28 usr/lib/libzpool.so.1`dnode_sync 2639 2607 44 42 lib/amd64/libtsol.so.2`__call_labeld 1039 1027 13 12 usr/sbin/snoop`detail_attr_bitmap 260 257 6 5 kernel/drv/amd64/vmxnet`Vxn_AllocInitBuffers 626 619 6 5 kernel/drv/amd64/mc-amd`rct_csintlv_bits 359 355 10 9 usr/lib/mdb/kvm/amd64/genunix.so`pfile_callback 1546 1529 34 33 kernel/drv/amd64/yge`yge_attach 1180 1168 18 14 usr/lib/libzpool.so.1`zfeature_register 409 405 6 5 kernel/fs/amd64/zfs`zil_lwb_write_issue 880 872 6 4 usr/lib/amd64/libsun_ima.so.1`setAuthMethods 346 343 4 2 kernel/drv/amd64/ixgbe`ixgbe_set_priv_prop 822 815 23 22 kernel/kmdb/amd64/genunix`stacks 3019 2995 55 51 usr/lib/mdb/proc/amd64/libc.so`stacks 3051 3027 54 50 usr/lib/mdb/kvm/amd64/genunix.so`stacks 3051 3027 54 50 usr/sbin/lofiadm`GetOptimum 5338 5297 76 75 kernel/drv/amd64/bnxe`lm_cmng_init 1429 1419 19 18 usr/lib/amd64/libzpool.so.1`zil_lwb_write_issue 1269 1261 11 9 kernel/kmdb/amd64/nca`nca_io2 1273 1265 14 12 usr/lib/mdb/kvm/amd64/nca.so`nca_io2 1313 1305 14 12 lib/amd64/libnsl.so.1`getipnodebyname 1453 1445 31 29 usr/bin/i86/truss`report 1409 1402 22 21 usr/bin/lex`packtrans 1999 1990 47 44 usr/lib/zones/zonestatd`zsd_refresh_memory 2708 2697 38 37 kernel/kmdb/amd64/genunix`pfile_callback 1275 1270 33 32 kernel/drv/amd64/pcic`pcic_setup_adapter 2602 2592 51 46 usr/bin/amd64/savecore`sendMTFValues 12636 12598 117 116 kernel/amd64/genunix`sendMTFValues 12768 12730 117 116 usr/lib/fm/amd64/libfmd_msg.so.1`fmd_msg_getitem_locked 1036 1033 13 12 usr/lib/zones/amd64/zoneadmd`add_net_for_linkid 1673 1669 20 19 lib/libc.so.1`__lc_collate_load 880 878 14 5 usr/sbin/isnsadm`subUsage 920 918 14 13 usr/sbin/stmfadm`subUsage 944 942 14 13 usr/sbin/sbdadm`subUsage 944 942 14 13 usr/sbin/sasinfo`subUsage 944 942 14 13 usr/sbin/fcinfo`subUsage 944 942 14 13 usr/sbin/fcadm`subUsage 944 942 14 13 usr/demo/comstar/bin/aluaadm`subUsage 944 942 14 13 lib/svc/method/svc-stmf`subUsage 944 942 14 13 lib/svc/method/iscsi-target`subUsage 944 942 14 13 kernel/drv/amd64/nv_sata`ck804_intr_process 1190 1188 21 17 usr/xpg6/bin/ls`pentry 2076 2074 37 36 usr/xpg4/bin/ls`pentry 2076 2074 37 36 usr/bin/ls`pentry 2076 2074 37 36
Of these, many (if not most) represent legitimate optimizations -- instances where the compiler unrolled a loop for which it can use the static size information to determine the number of iterations. For at least one class, though, -faggressive-loop-optimizations
resulted in deeply incorrect code: any code iterating over dn_nblkptr
in ZFS.
Take, for example, this code in dmu_objset_write_ready
:
for (int i = 0; i < dnp->dn_nblkptr; i++) bp->blk_fill += BP_GET_FILL(&dnp->dn_blkptr[i]);
Here is the generated code for this with -faggressive-loop-optimizations
:
dmu_objset_write_ready+0x27: f6 40 74 80 testb $0x80,0x74(%rax) dmu_objset_write_ready+0x2b: ba 01 00 00 00 movl $0x1,%edx dmu_objset_write_ready+0x30: 75 07 jne +0x7 <dmu_objset_write_ready+0x39> dmu_objset_write_ready+0x32: 48 8b 90 98 00 00 movq 0x98(%rax),%rdx 00 dmu_objset_write_ready+0x39: 48 89 53 58 movq %rdx,0x58(%rbx)
Note: no loop! This code should look something more like:
dmu_objset_write_ready+0x27: 48 8d 46 70 leaq 0x70(%rsi),%rax dmu_objset_write_ready+0x2b: 31 c9 xorl %ecx,%ecx dmu_objset_write_ready+0x2d: 31 d2 xorl %edx,%edx dmu_objset_write_ready+0x2f: 49 b8 00 00 00 00 movq $0x8000000000,%r8 80 00 00 00 dmu_objset_write_ready+0x39: 0f 1f 80 00 00 00 nopl 0x0(%rax) 00 dmu_objset_write_ready+0x40: 4c 85 00 testq %r8,(%rax) dmu_objset_write_ready+0x43: bf 01 00 00 00 movl $0x1,%edi dmu_objset_write_ready+0x48: 75 04 jne +0x4 <dmu_objset_write_ready+0x4e> dmu_objset_write_ready+0x4a: 48 8b 78 28 movq 0x28(%rax),%rdi dmu_objset_write_ready+0x4e: 48 01 f9 addq %rdi,%rcx dmu_objset_write_ready+0x51: ff c2 incl %edx dmu_objset_write_ready+0x53: 48 83 e8 80 subq $-0x80,%rax <0xffffffffffffff80> dmu_objset_write_ready+0x57: 48 89 4b 58 movq %rcx,0x58(%rbx) dmu_objset_write_ready+0x5b: 0f b6 7e 03 movzbl 0x3(%rsi),%edi dmu_objset_write_ready+0x5f: 39 d7 cmpl %edx,%edi dmu_objset_write_ready+0x61: 7f dd jg -0x23 <dmu_objset_write_ready+0x40>
The problem here is that dn_blkptr
is used somewhat craftily; it is declared to be an array of size 1 (that is, appearing to be a flexible array member), but in fact is used to index into dn_bonus
:
typedef struct dnode_phys { uint8_t dn_type; /* dmu_object_type_t */ uint8_t dn_indblkshift; /* ln2(indirect block size) */ uint8_t dn_nlevels; /* 1=dn_blkptr->data blocks */ uint8_t dn_nblkptr; /* length of dn_blkptr */ uint8_t dn_bonustype; /* type of data in bonus buffer */ uint8_t dn_checksum; /* ZIO_CHECKSUM type */ uint8_t dn_compress; /* ZIO_COMPRESS type */ uint8_t dn_flags; /* DNODE_FLAG_* */ uint16_t dn_datablkszsec; /* data block size in 512b sectors */ uint16_t dn_bonuslen; /* length of dn_bonus */ uint8_t dn_pad2[4]; /* accounting is protected by dn_dirty_mtx */ uint64_t dn_maxblkid; /* largest allocated block ID */ uint64_t dn_used; /* bytes (or sectors) of disk space */ uint64_t dn_pad3[4]; blkptr_t dn_blkptr[1]; uint8_t dn_bonus[DN_MAX_BONUSLEN - sizeof (blkptr_t)]; blkptr_t dn_spill; } dnode_phys_t;
The ZoL folks also ran into this ; their fix was to compile with -fno-aggressive-loop-optimizations
-- but they ultimately fixed the structure and then re-enabled the optimization . While this structure should definitely be changed to give the compiler a better idea of what's going on, it feels too risky to leave -faggressive-loop-optimizations
on (it is the default with -O2
). The win it delivers seems far too modest to pay for the risk of potential data corruption induced by incorrect code generation; we should compile with -fno-aggressive-loop-optimizations
.
We should disable this dangerous option across illumos.
This bug mirrors Joyent upstream bug OS-7517
Related issues
Updated by John Levon over 3 years ago
See https://smartos.org/bugview/OS-7517 for more details
Updated by John Levon over 3 years ago
- Related to Feature #9996: use GCC 7 as default primary compiler added
Updated by Joshua M. Clulow over 3 years ago
- Description updated (diff)
- Tags deleted (
needs-triage)
Updated by Joshua M. Clulow over 3 years ago
Additional comments from original ticket:¶
Bryan Cantrill added a comment - 16/Jan/19 11:42 AM¶
Two additional notes. First, the difference in has_saved_fp
is actually due to an array over-read; see OS-7518 (#10470). Second, vdev_raidz_reconstruct
(which troublingly appears in the list of potentially suspect functions) appears to have correct code generated with -faggressive-loop-optimizations
, and is in that regard apparently a false positive.
John Levon added a comment - 17/Jan/19 2:06 AM¶
Previously we thought that GCC would do this upon 1-sized last-member arrays (not just the dn_blkptr grossness). Was that mistaken, or is that still potentially the case? The reason I ask is that the GCC source in this area has special code for this, implying that if it is doing bad things, it would actually be considered a GCC Bug.
Bryan Cantrill added a comment - 22/Jan/19 9:11 AM¶
It appears that GCC is indeed handling the 1-sized last-member arrays properly (fortunately). The code generated is quite different (which led to the concern that loops were being obviated), but having gone over some of these functions (e.g. vdev_raidz_reconstruct
) quite carefully, it seems that the code is correct in this case, even in the presence of -faggressive-loop-optimizations
.
Updated by Electric Monk over 3 years ago
- Status changed from New to Closed
- % Done changed from 0 to 100
git commit 92c1a61163ff6a0655b27bd429856e171e7ce5f5
commit 92c1a61163ff6a0655b27bd429856e171e7ce5f5 Author: Bryan Cantrill <bryan@joyent.com> Date: 2019-03-01T11:11:54.000Z 10468 __ctype_mask[EOF] has been working by accident 10469 GCC's -faggressive-loop-optimizations is too aggressive 10470 array over-read in has_saved_fp() Reviewed by: Robert Mustacchi <rm@joyent.com> Reviewed by: John Levon <john.levon@joyent.com> Reviewed by: Toomas Soome <tsoome@me.com> Reviewed by: Igor Kozhukhov <igor@dilos.org> Reviewed by: Andy Fiddaman <andy@omniosce.org> Reviewed by: Gergő Doma <domag02@gmail.com> Reviewed by: Gary Mills <gary_mills@fastmail.fm> Approved by: Richard Lowe <richlowe@richlowe.net>