Apply the packed attribute to uint*_u types for Clang by MaskRay · Pull Request #1667 · samtools/htslib

MaskRay · 2023-08-25T00:08:57Z

... so that the following code

static inline void u32_to_le(uint32_t val, uint8_t *buf) {
    *((uint32_u *) buf) = val;
...

will not cause -fsanitize=alignment failures when building with Clang.

... so that the following code ``` static inline void u32_to_le(uint32_t val, uint8_t *buf) { *((uint32_u *) buf) = val; ... ``` will not cause -fsanitize=alignment failures when building with Clang.

daviesrob · 2023-08-25T16:03:16Z

clang seems happy enough with this, so I guess it's OK.

This hack was actually put in to prevent certain versions of gcc from using SIMD instructions that expected aligned data. If you want to use sanitizers, I'd recommend building with -DHTS_ALLOW_UNALIGNED=0 in CPPFLAGS which enables the properly standards-conforming version of the code. This will keep -fsanitize=undefined happy.

One day the dodgy typecasts will be replaced by calls to memcpy(), which is the proper way to do unaligned loads. At least these days most compilers manage to replace this with the intended register load (given sufficient optimisation).

MaskRay · 2023-08-25T21:12:49Z

Thank you!

If you want to use sanitizers, I'd recommend building with -DHTS_ALLOW_UNALIGNED=0 in CPPFLAGS which enables the properly standards-conforming version of the code. This will keep -fsanitize=undefined happy.

I think it'd be nice if users don't additionally specify -DHTS_ALLOW_UNALIGNED=0.

For further cleanup, functions like the following can just be replaced with memcpy. See Blosc/c-blosc2#550 (comment) that ((p)[0] | (p)[1]<<8 | (p)[2]<<16 | (p)[3]<<24) may not be optimized well while memcpy is always good:

static inline uint16_t le_to_u16(const uint8_t *buf) {
#if defined(HTS_LITTLE_ENDIAN) && HTS_ALLOW_UNALIGNED != 0
    return *((uint16_u *) buf);
#else
    return (uint16_t) buf[0] | ((uint16_t) buf[1] << 8);
#endif
}

Notice: this is the last SAMtools / HTSlib release where CRAM 3.0 will be the default CRAM version. From the next we will change to CRAM 3.1 unless the version is explicitly specified, for example using "samtools view -O cram,version=3.0". Updates ------- * Extend annot-tsv with several new command line options. --delim permits use of other delimiters. --headers for selection of other header formats. --no-header-idx to suppress column index numbers in header. Also removed -h as it is now short for --headers. Note --help still works. (PR samtools#1779) * Allow annot-tsv -a to rename annotations. (PR samtools#1709) * Extend annot-tsv --overlap to be able to specify the overlap fraction separately for source and target. (PR samtools#1811) * Added new APIs to facilitate low-level CRAM container manipulations, used by the new "samtools cat" region filtering code. Functions are: cram_container_get_coords() cram_filter_container() cram_index_extents() cram_container_num2offset() cram_container_offset2num() cram_num_containers() cram_num_containers_between() Also improved cram_index_query() to cope with HTS_IDX_NOCOOR regions. (PR samtools#1771) * Bgzip now retains file modification and access times when compressing and decompressing. (PR samtools#1727, fixes samtools#1718. Requested by Gert Hulselmans.) * Use FNV1a for string hashing in khash. The old algorithm was particularly weak with base-64 style strings and lead to a large number of collisions. (PR samtools#1806. Fixes samtools/samtools#2066, reported by Hans-Joachim Ruscheweyh) * Improve the speed of the nibble2base() function on Intel (PR samtools#1667, PR samtools#1764, PR samtools#1786, PR samtools#1802, thanks to Ruben Vorderman) and ARM (PR samtools#1795, thanks to John Marshall). * bgzf_getline() will now warn if it encounters UTF-16 data. (PR samtools#1487, thanks to John Marshall) * Speed up bgzf_read(). While this does not reduce CPU significantly, it does increase the maximum parallelism available permitting 10-15% faster decoding. (PR samtools#1772, PR samtools#1800, Issue samtools#1798) * Speed up faidx by use of better isgraph methods (PR samtools#1797) and whole-line reading (PR samtools#1799, thanks to John Marshall). * Speed up kputll() function, speeding up BAM -> SAM conversion by about 5% and also samtools depth. (PR samtools#1805) * Added more example code, covering fasta/fastq indexing, tabix indexing and use of the thread pool. (PR samtools#1666) Build Changes ------------- * Code warning fixes for pedantic compilers (PR samtools#1777) and avoid some undefined behaviour (PR samtools#1810, PR samtools#1816, PR samtools#1828). * Windows based CI has been migrated from AppVeyor to GitHub Actions. (PR samtools#1796, PR samtools#1803, PR samtools#1808) * Miscellaneous minor build infrastructure and code fixes. (PR samtools#1807, PR samtools#1829, both thanks to John Marshall) * Updated htscodecs submodule to version 1.6.1 (PR samtools#1828) * Fixed an awk script in the Makefile that only worked with gawk. (PR samtools#1831) Bug fixes --------- * Fix small OSS-Fuzz reported issues with CRAM encoding and long CIGARS and/or illegal positions. (PR samtools#1775, PR samtools#1801, PR samtools#1817) * Fix issues with on-the-fly indexing of VCF/BCF (bcftools --write-index) when not using multiple threads. (PR samtools#1837. Fixes samtools/bcftools#2267, reported by Giulio Genovese) * Stricter limits on POS / MPOS / TLEN in sam_parse1(). This fixes a signed overflow reported by OSS-Fuzz and should help prevent other as-yet undetected bugs. (PR samtools#1812) * Check that the underlying file open worked for preload: URLs. Fixes a NULL pointer dereference reported by OSS-Fuzz. (PR samtools#1821) * Fix an infinite loop in hts_itr_query() when given extremely large positions which cause integer overflow. Also adds hts_bin_maxpos() and hts_idx_maxpos() functions. (PR samtools#1774, thanks to John Marshall and reported by Jesus Alberto Munoz Mesa) * Fix an out of bounds read in hts_itr_multi_next() when switching chromosomes. This bug is present in releases 1.11 to 1.20. (PR samtools#1788. Fixes samtools/samtools#2063, reported by acorvelo) * Work around parsing problems with colons in CHROM names. Fixes samtools/bcftools#2139. (PR samtools#1781, John Marshall / James Bonfield) * Correct the CPU detection for Mac OS X 10.7. cpuid is used by htscodecs (see samtools/htscodecs#116), and the corresponding changes in htslib are PR samtools#1785. Reported by Ryan Carsten Schmidt. * Make BAM zero-length intervals work the same as CRAM; permitted and returning overlapping records. (PR samtools#1787. Fixes samtools/samtools#2060, reported by acorvelo) * Replace assert() with abort() in BCF synced reader. This is not an ideal solution, but it gives consistent behaviour when compiling with or without NDEBUG. (PR samtools#1791, thanks to Martin Pollard) * Fixed failure to change the write block size on compressed SAM or VCF files due to an internal type confusion. (PR samtools#1826) * Fixed an out-of-bounds read in cram_codec_iter_next() (PR samtools#1832)

htslib release 1.21: The primary user-visible changes in this release are updates to the annot-tsv tool and some speed improvements. Full details of other changes and bugs fixed are below. Notice: this is the last SAMtools / HTSlib release where CRAM 3.0 will be the default CRAM version. From the next we will change to CRAM 3.1 unless the version is explicitly specified, for example using "samtools view -O cram,version=3.0". Updates ------- * Extend annot-tsv with several new command line options. --delim permits use of other delimiters. --headers for selection of other header formats. --no-header-idx to suppress column index numbers in header. Also removed -h as it is now short for --headers. Note --help still works. (PR samtools#1779) * Allow annot-tsv -a to rename annotations. (PR samtools#1709) * Extend annot-tsv --overlap to be able to specify the overlap fraction separately for source and target. (PR samtools#1811) * Added new APIs to facilitate low-level CRAM container manipulations, used by the new "samtools cat" region filtering code. Functions are: cram_container_get_coords() cram_filter_container() cram_index_extents() cram_container_num2offset() cram_container_offset2num() cram_num_containers() cram_num_containers_between() Also improved cram_index_query() to cope with HTS_IDX_NOCOOR regions. (PR samtools#1771) * Bgzip now retains file modification and access times when compressing and decompressing. (PR samtools#1727, fixes samtools#1718. Requested by Gert Hulselmans.) * Use FNV1a for string hashing in khash. The old algorithm was particularly weak with base-64 style strings and lead to a large number of collisions. (PR samtools#1806. Fixes samtools/samtools#2066, reported by Hans-Joachim Ruscheweyh) * Improve the speed of the nibble2base() function on Intel (PR samtools#1667, PR samtools#1764, PR samtools#1786, PR samtools#1802, thanks to Ruben Vorderman) and ARM (PR samtools#1795, thanks to John Marshall). * bgzf_getline() will now warn if it encounters UTF-16 data. (PR samtools#1487, thanks to John Marshall) * Speed up bgzf_read(). While this does not reduce CPU significantly, it does increase the maximum parallelism available permitting 10-15% faster decoding. (PR samtools#1772, PR samtools#1800, Issue samtools#1798) * Speed up faidx by use of better isgraph methods (PR samtools#1797) and whole-line reading (PR samtools#1799, thanks to John Marshall). * Speed up kputll() function, speeding up BAM -> SAM conversion by about 5% and also samtools depth. (PR samtools#1805) * Added more example code, covering fasta/fastq indexing, tabix indexing and use of the thread pool. (PR samtools#1666) Build Changes ------------- * Code warning fixes for pedantic compilers (PR samtools#1777) and avoid some undefined behaviour (PR samtools#1810, PR samtools#1816, PR samtools#1828). * Windows based CI has been migrated from AppVeyor to GitHub Actions. (PR samtools#1796, PR samtools#1803, PR samtools#1808) * Miscellaneous minor build infrastructure and code fixes. (PR samtools#1807, PR samtools#1829, both thanks to John Marshall) * Updated htscodecs submodule to version 1.6.1 (PR samtools#1828) * Fixed an awk script in the Makefile that only worked with gawk. (PR samtools#1831) Bug fixes --------- * Fix small OSS-Fuzz reported issues with CRAM encoding and long CIGARS and/or illegal positions. (PR samtools#1775, PR samtools#1801, PR samtools#1817) * Fix issues with on-the-fly indexing of VCF/BCF (bcftools --write-index) when not using multiple threads. (PR samtools#1837. Fixes samtools/bcftools#2267, reported by Giulio Genovese) * Stricter limits on POS / MPOS / TLEN in sam_parse1(). This fixes a signed overflow reported by OSS-Fuzz and should help prevent other as-yet undetected bugs. (PR samtools#1812) * Check that the underlying file open worked for preload: URLs. Fixes a NULL pointer dereference reported by OSS-Fuzz. (PR samtools#1821) * Fix an infinite loop in hts_itr_query() when given extremely large positions which cause integer overflow. Also adds hts_bin_maxpos() and hts_idx_maxpos() functions. (PR samtools#1774, thanks to John Marshall and reported by Jesus Alberto Munoz Mesa) * Fix an out of bounds read in hts_itr_multi_next() when switching chromosomes. This bug is present in releases 1.11 to 1.20. (PR samtools#1788. Fixes samtools/samtools#2063, reported by acorvelo) * Work around parsing problems with colons in CHROM names. Fixes samtools/bcftools#2139. (PR samtools#1781, John Marshall / James Bonfield) * Correct the CPU detection for Mac OS X 10.7. cpuid is used by htscodecs (see samtools/htscodecs#116), and the corresponding changes in htslib are PR samtools#1785. Reported by Ryan Carsten Schmidt. * Make BAM zero-length intervals work the same as CRAM; permitted and returning overlapping records. (PR samtools#1787. Fixes samtools/samtools#2060, reported by acorvelo) * Replace assert() with abort() in BCF synced reader. This is not an ideal solution, but it gives consistent behaviour when compiling with or without NDEBUG. (PR samtools#1791, thanks to Martin Pollard) * Fixed failure to change the write block size on compressed SAM or VCF files due to an internal type confusion. (PR samtools#1826) * Fixed an out-of-bounds read in cram_codec_iter_next() (PR samtools#1832) # -----BEGIN PGP SIGNATURE----- # # iQJJBAABCgAzFiEEaGn1wQ1nqxSLDC2XHsvluhXGTMwFAmbjBlMVHHJtZCtnaXRA # c2FuZ2VyLmFjLnVrAAoJEB7L5boVxkzMgb0P/3AAxkJNLGK8Q1pBBo+in7KzwBOc # /nRfvReki47r4+i1ev98abEF/+XZoYSS8ZkiJH8JMkLCclb1ch2fDQTIL/sjJwrU # MfhYx6oHhzLUOjpG24m390wlvlkEsg5S69jbgImWxLQ1n4ZcSuZGumiwPx5274w5 # 3R/K9vIXtuSF9DTD/jrTgoZeBiN+B4XefAJqHLSo4/XzDo7DNMSZryNGFpHdyKcB # oEfEXX45vJLgzA3Fxo3LqNpC8UTDO5C9u9Uup+hCLN7RTBjV5Fu0UxcwWTSPqfiC # Jroi2FhxNPlDkRL3jraI8LrB3Cqb0G/31OU+cryasrqYnfJrl0e/MyXiRSa0sg25 # uHHWVyyC4JR7uBfUCppTVf76iajbkIVpHgHLZS1heGwM4PCL+AalvsIgGcvLXcBs # g/AwZLoXzao+pbzg8hzN1JuLB5hHcrHp6TsQ6JbTTrWaLeAL3s/+eQ4+d+Np2iUn # v8TMg6onu1HiNgVMQcFXSLC2Q51rB+lD6AA3eusKjcQsaxyk+U7SLCx3OhbvxGsd # SjxQWP1W+WW//e9nm5f4CTSSf1PHoDMyR/kVEc3qwEWJslri68RyB69HrDIVzrWT # 3gKAKJYafi5ssFHsKTsCngESpDsTg3aQV1AGOQPLN/TIjyYDTUOL+/HylRs3k9XN # KOGO5yggx+yqeL3w # =lHVl # -----END PGP SIGNATURE----- # gpg: Signature made Thu 12 Sep 2024 08:18:43 AM PDT # gpg: using RSA key 6869F5C10D67AB148B0C2D971ECBE5BA15C64CCC # gpg: issuer "rmd+git@sanger.ac.uk" # gpg: Can't check signature: No public key

Apply the packed attribute to uint*_u types for Clang

8f82e33

... so that the following code ``` static inline void u32_to_le(uint32_t val, uint8_t *buf) { *((uint32_u *) buf) = val; ... ``` will not cause -fsanitize=alignment failures when building with Clang.

daviesrob merged commit 7e20d76 into samtools:develop Aug 25, 2023

MaskRay deleted the clang branch August 25, 2023 21:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Apply the packed attribute to uint*_u types for Clang#1667

Apply the packed attribute to uint*_u types for Clang#1667
daviesrob merged 1 commit into
samtools:developfrom
MaskRay:clang

MaskRay commented Aug 25, 2023

Uh oh!

daviesrob commented Aug 25, 2023

Uh oh!

MaskRay commented Aug 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

MaskRay commented Aug 25, 2023

Uh oh!

daviesrob commented Aug 25, 2023

Uh oh!

MaskRay commented Aug 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants