String de-duplication for localtime_r

[tiif]

Since this is already brought up in #3470 (comment), I will just open a new issue for this.

To resolve the FIXME here,

miri/src/shims/time.rs

Lines 198 to 199 in 052bdcb

	// FIXME: String de-duplication is needed so that we only allocate this string only once
	// even when there are multiple calls to this function.

we could do:

We should have allocate_bytes which allocates a byte slice, and that should use the cache. allocate_str can call that, and transmute the result to &str.

[tiif]

@rustbot label +C-enhancement

[shamb0]

Hi @tiif, @RalfJung, could you please assign this issue to me? I’d like to work on it.

[saethlin]

Done.

[RalfJung]

Thanks for the PR! I am replying here so that the issue has at least a rough sketch of some mentoring instructions.

However, adding a new string cache is the wrong approach here. We already have a deduplication machinery in Miri: this.allocate_str. We don't want a second system doing the same thing. We definitely don't want static storing Pointer, that will go completely wrong if someone runs more than one Miri interpreter instance in the same process.

So this change will have to be done in stages:

• first a rustc PR that adds a new allocate_bytes, very similar to allocate_str that exist here. allocate_str should then use allocate_bytes.
• Then a Miri PR that changes the timezone logic to use allocate_bytes. (We don't really need the alloc_os_str_as_c_str stuff there since we start with a String for the timezone, not an OsString.)

[RalfJung]

No this has not been fixed yet...

[DianQK]

What is GitHub doing with my account? 🤔

[RalfJung]

It's because you pushed a commit that contains "closes: ..." to a master branch, and you have the permission to close issues here.

[saethlin]

This is a long-standing GifHub design flaw. The "fixes" magic commit messages are applied when you push to the default branch of a fork.

Most contributors "fix" this by not keeping their master branch up to date which is gross but better than randomly closing issues. I recently ran into the same problem and had to tweak my local workflow as a result. It's really unfortunate.

[DianQK]

Thanks. Very weird behavior. I have prevented accidental pushes by checking "Lock branch" and "Do not allow bypassing the above settings":

Enumerating objects: 3, done.
Counting objects: 100% (3/3), done.
Delta compression using up to 32 threads
Compressing objects: 100% (2/2), done.
Writing objects: 100% (2/2), 447 bytes | 447.00 KiB/s, done.
Total 2 (delta 1), reused 0 (delta 0), pack-reused 0 (from 0)
remote: Resolving deltas: 100% (1/1), completed with 1 local object.
remote: error: GH006: Protected branch update failed for refs/heads/master.
remote:
remote: - Cannot change this locked branch
To github.com:DianQK/rust.git
 ! [remote rejected]         master -> master (protected branch hook declined)
error: failed to push some refs to 'github.com:DianQK/rust.git'